| Message ID | 20180507073351.30582-7-jagan@amarulasolutions.com |
|---|---|
| State | Changes Requested |
| Delegated to: | Jagannadha Sutradharudu Teki |
| Headers | show |
| Series | phy: sunxi: Add Allwinner sun4i USB PHY | expand |
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@amarulasolutions.com> Why don't you implement a clock driver for this SoC instead ?
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@amarulasolutions.com> > > Why don't you implement a clock driver for this SoC instead ? Aren't you asking a bit too much? 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. 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. 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. Maxime
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@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. 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). 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. > 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. > 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. > 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 ? 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. 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.
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@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. > 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. > 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. > > 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. > > 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. > > 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. 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 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 :) Maxime
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@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 ?
Hi Marek, On Mon, May 7, 2018 at 5:17 PM, Marek Vasut <marex@denx.de> 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@amarulasolutions.com> > > Why don't you implement a clock driver for this SoC instead ? As of now we are unable to get pinctrl, clock and reset details from DT since the dm code on these will add it in future and the same I have mentioned in the cover-letter patch. Since Allwinner is big SOC's collection along with numerous amount of boards, and shortage of binary size (with few SOC's) we need to be careful while adding these features and which obviously taking some amount of time. Hope you understand. Jagan.
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@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 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. > >>> 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. This is no longer a simple request, but some huge spaghetti changes that need to be done, mostly by volunteers. 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. > >>> 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. > >> 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. Maxime
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@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).
On Sat, May 12, 2018 at 02:12:43PM +0200, Marek Vasut wrote: > >>>>> 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. rmobile? The scale isn't quite the same. It looks like there's 4 similar SoCs, with a dozen of boards supported. We have a dozen of SoCs supported, and around 120-130 boards. The clock tree looks much simpler too, and it seems like it has less drivers. And I don't really know what the constraints are on the SPL side, but it's really tight on our end. So maybe I'm exagerating, but you're definitely understating it too. > > 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. I have no idea to be honest. The maintainance however is volunteering on my side, and I'm getting a bit tired to see that every one has an agenda without any consideration about who has the time and resources to actually do it. > 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 ... I definitely understand and appreciate that, trust me. But the point here is that you were asking too much, so I guess my point is that you should spend *less* time reviewing stuff, which tends to make people less stressed :) Maxime
On 05/14/2018 11:05 AM, Maxime Ripard wrote: > On Sat, May 12, 2018 at 02:12:43PM +0200, Marek Vasut wrote: >>>>>>> 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. > > rmobile? The scale isn't quite the same. It looks like there's 4 > similar SoCs, with a dozen of boards supported. We have a dozen of > SoCs supported, and around 120-130 boards. The clock tree looks much > simpler too, and it seems like it has less drivers. Aha :) > And I don't really know what the constraints are on the SPL side, but > it's really tight on our end. So maybe I'm exagerating, but you're > definitely understating it too. You can fit into 16k , can you not ? >>> 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. > > I have no idea to be honest. The maintainance however is volunteering > on my side, and I'm getting a bit tired to see that every one has an > agenda without any consideration about who has the time and resources > to actually do it. My agenda is to make sure upstream doesn't become a swamp. >> 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 ... > > I definitely understand and appreciate that, trust me. But the point > here is that you were asking too much I was asking a question, is that too much nowadays ? I thought we cleared that point in this thread before. >, so I guess my point is that you > should spend *less* time reviewing stuff, which tends to make people > less stressed :) Not sure I understand this remark. I guess it doesn't fit my agenda.
On Mon, May 14, 2018 at 11:13:54AM +0200, Marek Vasut wrote: > > And I don't really know what the constraints are on the SPL side, but > > it's really tight on our end. So maybe I'm exagerating, but you're > > definitely understating it too. > > You can fit into 16k , can you not ? We have 13k. > >>> 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. > > > > I have no idea to be honest. The maintainance however is volunteering > > on my side, and I'm getting a bit tired to see that every one has an > > agenda without any consideration about who has the time and resources > > to actually do it. > > My agenda is to make sure upstream doesn't become a swamp. And it will become less of a swamp with those patches with the phy conversion, so you can tick that on your checklist I guess? Maxime
On 05/18/2018 01:51 PM, Maxime Ripard wrote: > On Mon, May 14, 2018 at 11:13:54AM +0200, Marek Vasut wrote: >>> And I don't really know what the constraints are on the SPL side, but >>> it's really tight on our end. So maybe I'm exagerating, but you're >>> definitely understating it too. >> >> You can fit into 16k , can you not ? > > We have 13k. Yuck, in total or for code/data ? Seems very similar to CI20. >>>>> 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. >>> >>> I have no idea to be honest. The maintainance however is volunteering >>> on my side, and I'm getting a bit tired to see that every one has an >>> agenda without any consideration about who has the time and resources >>> to actually do it. >> >> My agenda is to make sure upstream doesn't become a swamp. > > And it will become less of a swamp with those patches with the phy > conversion, so you can tick that on your checklist I guess? I think so
On Fri, May 18, 2018 at 01:55:31PM +0200, Marek Vasut wrote: > On 05/18/2018 01:51 PM, Maxime Ripard wrote: > > On Mon, May 14, 2018 at 11:13:54AM +0200, Marek Vasut wrote: > >>> And I don't really know what the constraints are on the SPL side, but > >>> it's really tight on our end. So maybe I'm exagerating, but you're > >>> definitely understating it too. > >> > >> You can fit into 16k , can you not ? > > > > We have 13k. > > Yuck, in total or for code/data ? > Seems very similar to CI20. It's the size of the SRAM, so total. Maxime
diff --git a/drivers/usb/musb-new/sunxi.c b/drivers/usb/musb-new/sunxi.c index af0dbc5a20..e79d7a2774 100644 --- a/drivers/usb/musb-new/sunxi.c +++ b/drivers/usb/musb-new/sunxi.c @@ -78,6 +78,8 @@ struct sunxi_musb_config { struct musb_hdrc_config *config; + u8 rst_bit; + u8 clkgate_bit; }; struct sunxi_glue { @@ -275,9 +277,16 @@ static int sunxi_musb_init(struct musb *musb) musb->isr = sunxi_musb_interrupt; setbits_le32(&glue->ccm->ahb_gate0, 1 << AHB_GATE_OFFSET_USB0); + if (glue->cfg->clkgate_bit) + setbits_le32(&glue->ccm->ahb_gate0, + 1 << glue->cfg->clkgate_bit); #ifdef CONFIG_SUNXI_GEN_SUN6I setbits_le32(&glue->ccm->ahb_reset0_cfg, 1 << AHB_GATE_OFFSET_USB0); + if (glue->cfg->rst_bit) + setbits_le32(&glue->ccm->ahb_reset0_cfg, + 1 << glue->cfg->rst_bit); #endif + sunxi_usb_phy_init(0); USBC_ConfigFIFO_Base(); @@ -408,8 +417,14 @@ static int musb_usb_remove(struct udevice *dev) sunxi_usb_phy_exit(0); #ifdef CONFIG_SUNXI_GEN_SUN6I clrbits_le32(&glue->ccm->ahb_reset0_cfg, 1 << AHB_GATE_OFFSET_USB0); + if (glue->cfg->rst_bit) + clrbits_le32(&glue->ccm->ahb_reset0_cfg, + 1 << glue->cfg->rst_bit); #endif clrbits_le32(&glue->ccm->ahb_gate0, 1 << AHB_GATE_OFFSET_USB0); + if (glue->cfg->clkgate_bit) + clrbits_le32(&glue->ccm->ahb_gate0, + 1 << glue->cfg->clkgate_bit); free(host->host); host->host = NULL; @@ -423,6 +438,8 @@ static const struct sunxi_musb_config sun4i_a10_cfg = { static const struct sunxi_musb_config sun8i_h3_cfg = { .config = &musb_config_h3, + .rst_bit = 23, + .clkgate_bit = 23, }; static const struct udevice_id sunxi_musb_ids[] = {
Add OTG device clkgate and reset for H3/H5 through driver_data. Signed-off-by: Jagan Teki <jagan@amarulasolutions.com> --- drivers/usb/musb-new/sunxi.c | 17 +++++++++++++++++ 1 file changed, 17 insertions(+)