On 10/11/2023 16:12, Daniel Golle wrote: >>>>>>>>> diff --git a/include/dt-bindings/reset/mediatek,mt7988-resets.h b/include/dt-bindings/reset/mediatek,mt7988-resets.h >>>>>>>>> new file mode 100644 >>>>>>>>> index 0000000000000..fa7c937505e08 >>>>>>>>> --- /dev/null >>>>>>>>> +++ b/include/dt-bindings/reset/mediatek,mt7988-resets.h >>>>>>>>> @@ -0,0 +1,12 @@ >>>>>>>>> +/* SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) */ >>>>>>>>> + >>>>>>>>> +/* TOPRGU resets */ >>>>>>>> >>>>>>>> The first reset is zero, the second reset is one. >>>>>>>> >>>>>>>> Where's the zero'th reset? :-) >>>>>>> >>>>>>> Currently the reset numbers represent the corresponding bit positions in >>>>>>> the toprgu register, as this is how the mtk-wdt driver is organized. >>>>>>> >>>>>>> So there is probably something at bit 0, and also at bit 3~11 and >>>>>>> maybe also 17~23, but it's unknown and may be added later once known >>>>>>> and/or needed. >>>>>> >>>>>> There is no need to put register bits, which are not used by the driver, >>>>>> in the bindings. >>>>> >>>>> There aren't. That's why there isn't a zero'th reset (and also not 3~11, 17~24). >>>>> >>>>> Or should the driver be reorganized to provide a mapping of logical to >>>>> physical resets, and then have only the needed once present and start >>>>> counting logical resets from 0? This is doable, of course, but it's a >>>>> bit of effort just for the aesthetical goal of starting to count from >>>>> zero and continous in header file. >>>>> >>>>> And, of course, chances are that other currently still unused bits >>>>> will be needed at a later point which then would mean having to add >>>>> them in at least 2 places (header file and mapping logical<->physical) >>>>> where as currently it would just mean adding a line defining it in the >>>>> header file. >>>> >>>> You can do it, but it's not what I wrote here. So bear with me: >>>> >>>> "There is no need to put register bits in the bindings." >> >> No comments here, so I assume you agree with this. Here is the answer to... >> >>>> >>>> You replied "There aren't", which I don't understand in this context. I >>>> can be clearer: >>>> Drop this hunk. >>> >>> 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. 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. Best regards, Krzysztof
