Hello Sean, Geert, On Tue, 10 Sep 2024 17:13:55 -0500 Rob Herring <robh+dt@xxxxxxxxxx> wrote: > On Wed, Mar 22, 2023 at 3:39 AM Luca Ceresoli <luca.ceresoli@xxxxxxxxxxx> wrote: > > > > Hello Stephen, > > > > On Mon, 20 Mar 2023 14:27:56 -0700 > > Stephen Boyd <sboyd@xxxxxxxxxx> wrote: > > > > > Quoting Sean Anderson (2023-01-24 08:23:45) > > > > On 1/24/23 03:28, Geert Uytterhoeven wrote: > > > > > Hi Luca, > > > > > > > > > > On Tue, Jan 24, 2023 at 9:12 AM Luca Ceresoli <luca.ceresoli@xxxxxxxxxxx> wrote: > > > > >> On Thu, 19 Jan 2023 14:27:43 -0500 > > > > >> Sean Anderson <sean.anderson@xxxxxxxx> wrote: > > > > >> > On 1/11/23 10:55, Geert Uytterhoeven wrote: > > > > > > > > > >> I'm wondering whether Geert has a practical example of a situation > > > > >> where it is better to have these properties optional. > > > > > > > > > > My issue was that these properties were introduced long after the > > > > > initial bindings, hence pre-existing DTS does not have them. > > > > > Yes, we can add them, but then we have to read out the OTP-programmed > > > > > settings first. If that's the way to go, I can look into that, though... > > > > > > > > FWIW I think there's no need to update existing bindings which don't > > > > have this property. The required aspect is mainly a reminder for new > > > > device trees. > > > > > > > > > > Is there any resolution on this thread? I'm dropping this patch from my > > > queue. > > > > IIRC Geert kind of accepted the idea that these properties should stay > > required. Which is a bit annoying but it's the safest option, so unless > > there are new complaints with solid use cases for making them optionalm, > > I think it's OK to drop the patch. > > The warnings related to this are now at the top of the list (by number > of occurrences): > > 50 clock-generator@6a: 'idt,shutdown' is a required property > 50 clock-generator@6a: 'idt,output-enable-active' is a required property > > IMO, if these properties haven't been needed for years, then they > obviously aren't really required. I think Rob's point adds to Geert's observation that there are other "idt,*" properties in the output nodes that may also be important to have correctly set, and are optional. So, Sean, I understand when you state it's safer to have these set. However this is valid for lots of other optional properties in any binding. Optional properties _can_ be set if that's important, just it's not mandatory to set them in all cases. As a matter of fact, we have been having for a long time some in-tree device trees which don't set these properties, which I believe implies it's OK for those cases to not set them, and to let them be set for the device trees where it is important. Finally, there is a maintenance/legacy issue: if we wanted to keep these properties optional, who would chase all the boards defined in existing device trees to discover the correct values? Bottom line, my Reviewed-by tag is still valid. What is your opinion given these last few discussion point Sean? Luca -- Luca Ceresoli, Bootlin Embedded Linux and Kernel engineering https://bootlin.com
