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 <ja...@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 ? >> 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 ? >> I think that if we are doing some sort of conversion, we should do it >> completely and properly instead of leaving in hacks like this. A clock >> driver which allows enabling/disabling clock is probably like what, 2 >> hour work ? So maybe it's worth investing that time up front to save >> maintenance burden in the future. > > This is definitely not a 2 hours job. More like 2 weeks if you want to > do it properly, and by which I mean creating the clock driver, > converting all the users to it, and then making sure you don't have > any regressions. > > This is on our radar, but this won't happen overnight. Fine >>> 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. >>> Jagan complied to all your requests so far, but this one is going to >>> create yet another ton of patches on top of an (already) 35 patches >>> series. And this request comes out of nowhere at the 7th version. >> >> I disagree, one clock driver patch and a tweak to the series, unless I >> missed something obvious. > > This won't be one clock driver patch. Seriously. Fine >>> 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. >> 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 ? -- Best regards, Marek Vasut _______________________________________________ U-Boot mailing list U-Boot@lists.denx.de https://lists.denx.de/listinfo/u-boot