On 10/11/2023 18:07, Daniel Golle wrote: > On Fri, Nov 10, 2023 at 04:21:35PM +0100, Krzysztof Kozlowski wrote: >> On 10/11/2023 16:15, Krzysztof Kozlowski wrote: >>>>>> So adding the file to include/dt-bindings/reset/ should go into a >>>>>> seperate patch? Because including it with the driver itself gave me >>>>>> a checkpath warning telling me that dt-bindings should go seperate, >>>>>> which is why I included it with the binding docs. >>>>> >>>>> No, I said the hunk should be dropped. Removed. >>>> >>>> I guess we are somehow misunderstanding each other. >>>> Lets go with an example. I can put the header into a commit of its own, >>>> just like commit >>>> 5794dda109fc8 dt-bindings: reset: mt7986: Add reset-controller header file >>>> https://lore.kernel.org/r/20220105100456.7126-2-sam.shih@xxxxxxxxxxxx >>>> >>>> Would that be acceptable? And if not, why? >>> >>> ...this question. > > ... which you didn't answer. Sorry, but it's not helpful to be polemic > or ironic in a code review involving non-native English speakers > trying to understand each others. Why do you keep skipping - third time - my earlier reply? The earlier paragraph? Cutting lines out of context is not how this should be discussed. So let me bring it: >> FOO > Here is the answer to... >> BAR > ...this question. The "FOO" Is the answer. Go open the emails and read the answer. > >>> >>> Again, whether this is separate patch - it is still hunk which I think >>> should be removed. I gave the reason "why" in this mail thread and in >>> multiple other discussions. >> >> I gave you clear reasoning 7 hours ago: >> https://lore.kernel.org/all/59629ec1-cc0c-4c5a-87cc-ea30d64ec191@xxxxxxxxxx/ >> to which you did not respond. > > Because it doesn't match anything existing regarding MediaTek reset > drivers, and I was assuming there must be some kind of misunderstanding, > which is why I replied to your later email in the same thread. > > My assumption that the problem was merely having documentation and > header combined in a single commit stems from the fact that a very > similar patch for MT7986[1] was Ack'ed by Rob Herring about a year and > a half ago; hence the rule you apply here may have always existed, but > apparently then hasn't been applied in the past. > > Literally *all* existing dt-binding headers for MediaTek SoCs follow a > direct 1:1 mapping of reset bit in hardware and reset number in the > header files. The driver is simple, all it cares about is the maximum > number defined in the header (and I like that, because it makes it very > easy to add new SoCs). At this point the abstraction needed to > fulfill your request doesn't exist, not for any of the SoCs using > mtk_wdt.c. It can be implemented, surely, it's a problem computers can > solve. If that's what you (and current maintainers of that driver) > would want me to implement, please say so clearly and spell it out. > > Also be clear about if all the other existing headers need to be > converted, mappings for all SoCs created in the driver, ... all before > support for MT7988 can go in? > Or should the existing headers for other MediaTek SoCs remain > untouched because they are already considered stable API or something? I am repeating myself... but I don't know how to put it other way. I did not ask you to rewrite your driver. I asked to drop the change to bindings, because it is entirely pointless. Drop this change, only this. No need to rewrite drivers, they stay the same. Best regards, Krzysztof
