On 10/29/21 00:24, Rob Herring wrote: > On Wed, Oct 20, 2021 at 01:06:13PM +0200, Marek Vasut wrote: >> On 10/20/21 12:14 PM, Vaittinen, Matti wrote: >> [...] >> >>> I wonder if this really is something specific to ROHM ICs? Do you think >>> this would warrant a generic, non vendor specific property? I am Ok with >>> the ROHM specific property too but it just seems to me this might not be >>> unique to ROHM IC(s). > > I imagine we debated the need for a DT property when critical clocks was > added to the kernel. Sorry. I guess I've missed this. Maybe this was done back when I spent my days tinkering with strictly proprietary systems - or then I have just missed it. >>> By the way, the very same clk driver where you implemented the property >>> reading (patch 2/2) is used by few other ROHM PMICs. At least by >>> BD71837, BD71828, BD71815, BD9576 and BD9573. So the code change here >>> adds support for this property to all of those PMICs. I wonder if the >>> property should be mentioned in all of the binding docs... That could be >>> another argument for making this a generic property and describing it in >>> clk yaml ;) >>> >>> Well, just my 10 Cents - I am ok with this change as you presented it >>> here if you don't think this should be generic one. >> >> I think we need something like gpio-hog, except for clock. Some clk-hog >> maybe ? That would be useful not only here, but also for things where some >> output generates clock for random stuff which cannot be described in the DT >> for whatever reason (like e.g. the SoC is used as a substitute for CPLD XTAL >> and the CPLD isn't connected to the SoC in any other way). > > The justification given in this patch was for an SoC input which should > get described so that the clock is handled and kept enabled properly. > > The CPLD case would be more interesting, but is there an actual need or > just a possible case? > > You could use the 'protected-clocks' property here. Maybe that's a bit > overloaded between can't access and don't turn off. But what it means is > really up the clock controller. I think I have seen some patch series which are aimed to providing common implementation for the 'protected-clocks'. It seems to me the intended 'protected-clocks' handling is simply not registering these clocks. I don't see this handling in-tree yet though and I did not find any discussion as to why the generic support has not been merged. So, if the 'protected-clocks' is to be supported by the driver, then I wonder if the handling should be 'ensure enabled and don't register to clock core' or plain 'don't register to clock core'? Best Regards --Matti Vaittinen
