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
