Re: [PATCH v10 4/5] counter: Add Renesas RZ/G2L MTU3a counter driver

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hi Biju,

On Fri, Dec 16, 2022 at 9:50 PM Biju Das <biju.das.jz@xxxxxxxxxxxxxx> wrote:
> Add RZ/G2L MTU3a counter driver. This IP supports the following
> phase counting modes on MTU1 and MTU2 channels
>
> 1) 16-bit phase counting modes on MTU1 and MTU2 channels.
> 2) 32-bit phase counting mode by cascading MTU1 and MTU2 channels.
>
> This patch adds 3 counter value channels.
>         count0: 16-bit phase counter value channel on MTU1
>         count1: 16-bit phase counter value channel on MTU2
>         count2: 32-bit phase counter value channel by cascading
>                 MTU1 and MTU2 channels.
>
> The external input phase clock pin for the counter value channels
> are as follows:
>         count0: "MTCLKA-MTCLKB"
>         count1: "MTCLKA-MTCLKB" or "MTCLKC-MTCLKD"
>         count2: "MTCLKA-MTCLKB" or "MTCLKC-MTCLKD"
>
> Use the sysfs variable "external_input_phase_clock_select" to select the
> external input phase clock pin and "cascade_counts_enable" to enable/
> disable cascading of channels.
>
> Signed-off-by: Biju Das <biju.das.jz@xxxxxxxxxxxxxx>
> ---
> v9->v10:

Thanks for the update!

> --- /dev/null
> +++ b/drivers/counter/rz-mtu3-cnt.c

> +static int rz_mtu3_count_read(struct counter_device *counter,
> +                             struct counter_count *count, u64 *val)
> +{
> +       struct rz_mtu3_channel *const ch = rz_mtu3_get_ch(counter, count->id);
> +       struct rz_mtu3_cnt *const priv = counter_priv(counter);
> +
> +       mutex_lock(&priv->lock);
> +       if (ch->is_busy && !priv->count_is_enabled[count->id]) {
> +               mutex_unlock(&priv->lock);
> +               return -EINVAL;
> +       }
> +
> +       if (rz_mtu3_is_counter_invalid(counter, count->id)) {
> +               mutex_unlock(&priv->lock);
> +               return -EBUSY;
> +       }

As the locking and the above two checks are duplicated multiple times,
perhaps they can be replaced by an rz_mtu3_lock_if_counter_is_valid()
helper function?

> +static int rz_mtu3_count_function_read(struct counter_device *counter,
> +                                      struct counter_count *count,
> +                                      enum counter_function *function)
> +{
> +       struct rz_mtu3_channel *const ch = rz_mtu3_get_ch(counter, count->id);
> +       struct rz_mtu3_cnt *const priv = counter_priv(counter);
> +       int ret;
> +
> +       mutex_lock(&priv->lock);
> +       if (ch->is_busy && !priv->count_is_enabled[count->id]) {
> +               mutex_unlock(&priv->lock);
> +               return -EINVAL;
> +       }

rz_mtu3_lock_if_count_is_disabled() helper?
(can also be called by rz_mtu3_lock_if_counter_is_valid())

> +static int rz_mtu3_cascade_counts_enable_set(struct counter_device *counter,
> +                                            u8 cascade_enable)
> +{
> +       struct rz_mtu3_cnt *const priv = counter_priv(counter);
> +
> +       mutex_lock(&priv->lock);
> +       if (priv->ch->is_busy && !rz_mtu3_is_ch0_enabled(priv)) {
> +               mutex_unlock(&priv->lock);
> +               return -EINVAL;
> +       }

rz_mtu3_lock_if_count_is_disabled() helper?

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



[Index of Archives]     [Linux Samsung SOC]     [Linux Wireless]     [Linux Kernel]     [ATH6KL]     [Linux Bluetooth]     [Linux Netdev]     [Kernel Newbies]     [IDE]     [Security]     [Git]     [Netfilter]     [Bugtraq]     [Yosemite News]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux ATA RAID]     [Samba]     [Device Mapper]

  Powered by Linux