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 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

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.