On Sat, 27 May 2023 00:16:36 +0300 Andy Shevchenko <andy.shevchenko@xxxxxxxxx> wrote: > On Thu, May 25, 2023 at 6:24 PM Jonathan Cameron > <Jonathan.Cameron@xxxxxxxxxx> wrote: > > > > Needs tidying up - hopefully can do clock right > > using on going work from Niyas > > https://linaro.atlassian.net/wiki/spaces/CLIENTPC/pages/28832333867/ACPI+Clock+Management > > For the current code base the easiest way is to switch to _optional > for clock, or request them based on the type of the fwnode. (Personal > preference is the _optional() API to call). Absolutely agree that would the way to go if people want to support my crazy. However, that will leave the input clock frequency unknown which means we'll program a garbage value into one of the device registers. Doesn't matter to me, but not good in general. This is avoiding for now the questions of: 1) Why devm for a clock we hold for 2 lines of code, none of which have an error return path... 2) clk_get_rate() is documented as not guaranteed to do anything for a clk until enabled, so this is relying on it being enabled by someone else or a quirk of the the chip. > For the reset isn't it > transparent already so we got a dummy control (as for regulator)? I don't think so, but maybe I'm missing something. There is a devm_reset_control_get_optional() though, similar to the clock one that returns a NULL if not present. I'll use that here to make this a slightly less ugly hack. If I can handle clocks nicely using Niyas' work then can revisit whether the i2c and aspeed maintainers would accept making the reset optional. Jonathan > >
