On 10/29/21 00:24, Rob Herring wrote:

On Wed, Oct 20, 2021 at 01:06:13PM +0200, Marek Vasut wrote:

quoted

On 10/20/21 12:14 PM, Vaittinen, Matti wrote:
[...]

quoted

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.

quoted

quoted

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