[U-Boot] [PATCH v7 06/35] musb: sunxi: Add OTG device clkgate and reset for H3/H5

[Marek Vasut]

On 05/11/2018 11:29 PM, Maxime Ripard wrote:
> On Mon, May 07, 2018 at 10:55:16PM +0200, Marek Vasut wrote:
>> On 05/07/2018 10:11 PM, Maxime Ripard wrote:
>>> On Mon, May 07, 2018 at 05:32:34PM +0200, Marek Vasut wrote:
>>>> On 05/07/2018 04:52 PM, Maxime Ripard wrote:
>>>>> On Mon, May 07, 2018 at 01:47:43PM +0200, Marek Vasut wrote:
>>>>>> On 05/07/2018 09:33 AM, Jagan Teki wrote:
>>>>>>> Add OTG device clkgate and reset for H3/H5 through driver_data.
>>>>>>>
>>>>>>> Signed-off-by: Jagan Teki <jagan at amarulasolutions.com>
>>>>>>
>>>>>> Why don't you implement a clock driver for this SoC instead ?
>>>>>
>>>>> Aren't you asking a bit too much?
>>>>
>>>> I am not asking for anything, this is a question, not a request.
>>>
>>> My bad then, this definitely sounded like a request to me.
>>
>> So uh, how do I make this NOT sound like a request to you ?
>> Can you phrase it for me ?
> 
> You are in a situation of power here. Asking the exact same question
> when you are the one in power or is a peer doesn't have the same
> impact, unless you make it clear that it is an actual question and not
> some way to have it fixed.
> 
> Something like "Would switching to a proper clock driver be an
> option?" for example would have carried the message better imo, if
> this was a genuine question and not a request.

I have to admit, I prefer simple, frugal, direct and clear questions
which can be answered equally clearly rather than long essays filled
with verbal fluff.

I'll consider this, but given that I am not a native English speaker, I
cannot guarantee the result to be to everyone's satisfaction.

>>>> I asked why not implement a clock driver and use it just like any other
>>>> civilized modern driver would instead of digging in the clock controller
>>>> registers from a USB framework driver (which is icky).
>>>
>>> From an absolute point of view, I agree. But we are where we are.
>>
>> Which is where exactly ?
> 
> Having to deal with code from 2012 everywhere.

This sounds like a massive generalization and an incorrect one to me *.

>>>>> Since the first post of these patches, you've asked to rework in a
>>>>> significant manner the driver already, including doing a new PHY
>>>>> driver to use the device model, and making other substantial changes
>>>>> to it.
>>>>
>>>> Well yes, because it was crap at the beginning and I don't want to see
>>>> the crap accumulating. It has become much better since, as you can see I
>>>> only had a few minor comments.
>>>
>>> And that's totally your role, but at the same time, the point of this
>>> series is not to fix the whole world, but rather add support for one
>>> particular SoC that is using pretty much the same design than any of
>>> our other SoCs' USB phy before. And here we are, 35 patches and
>>> counting.
>>
>> If I said "yes" to every single patch adding just a minor additional bit
>> of crap to the codebase, we'd be in the state in which we were in 2012,
>> sinking under the boatload of ifdeffery and ad-hoc solutions. So I think
>> some push is needed to avoid that situation.
> 
> I don't have any issue with the end goal, and your willingness to have
> the code ported over to new APIs. But if from one day to another every
> maintainer goes like this, this will simply not fly. This is not just
> about having just a simple clock driver, but also a pinctrl one, and
> converting all the consumer drivers to the device model, oh, and btw,
> the DM doesn't fit in the SPL anymore, so we would probably need to do
> an SPL driver as well. Probably with some painful Kconfig conversions
> all over the tree even.

You are massively exaggerating right there. I recently did such a
conversion for a platform and it didn't take nearly as much effort as
you describe and/or it could be well segmented.

> This is no longer a simple request, but some huge spaghetti changes
> that need to be done, mostly by volunteers.

I am not sure this "volunteers" argument really works in this
discussion, since this looks like a commercial contribution to me.

But if you want to discuss volunteering, did you ever consider that I
also do the USB maintaining in my free time and the bulk of
communication is random people demanding random stuff ? I also don't see
people coming up saying "oh, hey, I'll spend some of my own free time to
help out maintaining this piece of code". It tends to make people
stressed and burnt out ...

> And at some point, it just
> becomes easier to give up, fork, and just maintain our stuff like we
> were doing before. Or just stop maintaining it entirely. And I'm not
> sure either situation is something we want.
> 
> tl; dr: I'd like some moderation.

So much dramatization for a simple question which could've had equally
simple answer, really.

>>>>> Creating a new clock driver will take a lot of effort, and this really
>>>>> surprise me given that we've had strictly no feedback from you on this
>>>>> considering all the previous SoCs bringups we've done so far.
>>>>
>>>> What do you mean by "this" ? I think i did review the previous
>>>> iterations of this series ? If not, was I on CC ?
>>>
>>> You did, and thanks a lot for that. The only thing I'm noting is that
>>> it's the first time you're being so picky about a series.
>>
>> Er, no, I am always picky and hard.
>>
>>> I appreciate that you have to draw the line somewhere, and when
>>> things you want in your subsystem aren't moving as fast as you'd
>>> like them to be you have to enforce new rules. But if you were
>>> unhappy about something, you never told us, which doesn't seem
>>> like a good path forward either. Even in your previous reviews of
>>> that particular series.
>>
>> I think I pointed out pretty much all of it ? If I missed something,
>> it's because it was hidden and didn't surface until the patchset got
>> into some better shape.
> 
> And yet, this is the first time you bring up the phy and clock
> support.

If I missed something, it's because it was hidden and didn't surface
until the patchset got into some better shape.

>>>> I have to admit, I don't really care about the rest of the Allwinner SoC
>>>> code or what you do there, I only care about the USB part and this
>>>> poking of clock controller registers seems wrong in a DM/DT driver.
>>>
>>> And I do agree on that. But we also have some history to carry.
>>>
>>>> I also don't mind if the clock driver comes later, but I would like to
>>>> see it happen at some point (soon) to remove this register poking.
>>>
>>> Awesome then :)
>>
>> Is this going to happen at some point ?
> 
> At some point, yes, but I can't give you a deadline.

That usually means never, sadly (and see * above).

-- 
Best regards,
Marek Vasut