Re: [PATCH RFC 2/2] regulator: raa215300: add support for configurable 32kHz clock output

[Josua Mayer <josua@solid-run.com>]

Am 03.05.26 um 16:49 schrieb Josua Mayer:
> Am 03.05.26 um 02:57 schrieb Mark Brown:
>> On Sat, May 02, 2026 at 06:07:05PM +0200, Josua Mayer wrote:
>>> Renesas RA215300 PMIC can be configured to output a 32kHz clock on its
>>> multi-purpose MPIO2 pin.
>>> There are in total 6 configurable multi-purpose pins, however only one
>>> of them supports outputting a clock in one specific configuration.
>> So there should be some pinmux support here then?  This is starting to
>> sound like a MFD...
> If we want to treat it like an MFD, then the logical sub-devices would be:
>
> 1. pinmux/pinconf
> 2. clock
> 3. gpi, gpo

I forgot about two more functions:

4. reset: software trigger to reset system (reset-output), e.g. for reboot
5. poweroff: software trigger for power button, e.g. for shutdown

>
> Then there is the RTC, which is not a sub-device because it has its own i2c
> bus address, but its power controls inside the raa215300.
>
> And there are some more complex regulator status and configuration registers.
>
> Implemented is only the RTC ... and I would like to add the clock,
> as it is used for Bluetooth on a SolidRun board.
>
> My own particular use-case would also be satisfied by implementing
> pinmux instead, as I need the 32kHz rate which is default.
>
>>> +#define RAA215300_MPIO2_POWER_OFF_DELAY			GENMASK(6, 0)
>>> +#define RAA215300_REG_MPIO2_CONFIG			0x8c
>>> +static void raa215300_clk_unprepare(struct clk_hw *hw)
>>> +{
>>> +	struct raa215300_clk *clk = to_raa215300_clk(hw);
>>> +	const u8 dis_val = RAA215300_MPIO2_CONFIG_TYPE_HIGH_IMPEDANCE |
>>> +			   RAA215300_MPIO2_CONFIG_FUNCTION_NONE;
>>> +
>>> +	regmap_write(clk->regmap, RAA215300_REG_MPIO2_CONFIG, dis_val);
>>> +}
> Considering I use the pin configuration register to enable and disable the clock,
> one might argue that if a pinmux driver exists, then the clock is always on
> and does not support prepare/unprepare.
>
> This would allow me to skip implemeting a clock subdevice,
> and instead only implement a pinconf/mux driver.
>
> Any opinions?
>
>>> +static unsigned long raa215300_clk_recalc_rate(struct clk_hw *hw, unsigned long parent_rate)
>>> +{
>>> +	struct raa215300_clk *clk = to_raa215300_clk(hw);
>>> +	unsigned int val;
>>> +
>>> +	regmap_read(clk->regmap, RAA215300_REG_MPIO2_POWER_OFF, &val);
>>> +	val &= RAA215300_MPIO2_POWER_OFF_DELAY;
>>> +
>>> +	return 32768 >> val;
>>> +}
>> Given the mask above val could be up to 127?  If nothing else it'd be
>> good to have some validation.
> Does it need validation if the mask is good?
>>> +		/* register mpio2 32k clkout in common clk framework */
>>> +		raa215300_register_clk(dev, regmap);
>> You should check the return value here.
> Ack.