Hi Biju,
On Thu, Nov 24, 2022 at 6:00 PM Biju Das <biju.das.jz@xxxxxxxxxxxxxx> wrote:
> The RZ/G2L multi-function timer pulse unit 3 (MTU3a) is embedded in
> the Renesas RZ/G2L family SoCs. It consists of eight 16-bit timer
> channels and one 32-bit timer channel. It supports the following
> functions
> - Counter
> - Timer
> - PWM
>
> The 8/16/32 bit registers are mixed in each channel.
>
> Add MTU3a core driver for RZ/G2L SoC. The core driver shares the
> clk and channel register access for the other child devices like
> Counter, PWM, Clock Source, and Clock event.
>
> Signed-off-by: Biju Das <biju.das.jz@xxxxxxxxxxxxxx>
> ---
> v6->v7:
> * Added channel specific mutex to avoid races between child devices
> (for eg: pwm and counter)
> * Added rz_mtu3_shared_reg_update_bit() to update bit.
Thanks for the update!
> --- /dev/null
> +++ b/drivers/clocksource/rz-mtu3.c
> +void rz_mtu3_shared_reg_update_bit(struct rz_mtu3_channel *ch, u16 off,
> + u16 pos, u8 val)
> +{
> + unsigned long tmdr;
> +
> + tmdr = rz_mtu3_shared_reg_read(ch, off);
> + __assign_bit(pos, &tmdr, !!val);
> + rz_mtu3_shared_reg_write(ch, off, tmdr);
As this is RMW on a shared register, probably this needs locking,
here or in the callers?
> +}
> +EXPORT_SYMBOL_GPL(rz_mtu3_shared_reg_update_bit);
> --- /dev/null
> +++ b/include/clocksource/rz-mtu3.h
> +enum rz_mtu3_functions {
> + RZ_MTU3_NORMAL,
IIRC, this state means the channel is idle?
RZ_MTU3_IDLE?
> + RZ_MTU3_16BIT_PHASE_COUNTING,
> + RZ_MTU3_32BIT_PHASE_COUNTING,
> + RZ_MTU3_PWM_MODE_1,
Do you need to know the actual function, or is it sufficient to just
know if the channel is busy? ...
> +};
> +
> +/**
> + * struct rz_mtu3_channel - MTU3 channel private data
> + *
> + * @dev: device handle
> + * @index: channel index
> + * @base: channel base address
> + * @lock: Lock to protect channel function
> + * @function: channel function
> + */
> +struct rz_mtu3_channel {
> + struct device *dev;
> + unsigned int index;
> + void __iomem *base;
> + struct mutex lock; /* Protect channel function */
> + enum rz_mtu3_functions function;
... so perhaps a simple busy flag is sufficient?
Regardless, to avoid replicating the channel function in every
subdriver, I would add a simple API (can be inline) to request and
release a channel.
> +};
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@xxxxxxxxxxxxxx
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
