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]

 




> -----Original Message-----
> From: Biju Das <biju.das.jz@xxxxxxxxxxxxxx>
> Sent: 09 January 2023 14:28
> To: Geert Uytterhoeven <geert@xxxxxxxxxxxxxx>
> Cc: William Breathitt Gray <william.gray@xxxxxxxxxx>; linux-
> iio@xxxxxxxxxxxxxxx; Chris Paterson <Chris.Paterson2@xxxxxxxxxxx>; Prabhakar
> Mahadev Lad <prabhakar.mahadev-lad.rj@xxxxxxxxxxxxxx>; linux-renesas-
> soc@xxxxxxxxxxxxxxx
> Subject: RE: [PATCH v10 4/5] counter: Add Renesas RZ/G2L MTU3a counter
> driver
> 
> Hi Geert,
> 
> Thanks for the feedback.
> 
> > -----Original Message-----
> > From: Geert Uytterhoeven <geert@xxxxxxxxxxxxxx>
> > Sent: 19 December 2022 08:40
> > To: Biju Das <biju.das.jz@xxxxxxxxxxxxxx>
> > Cc: William Breathitt Gray <william.gray@xxxxxxxxxx>; linux-
> > iio@xxxxxxxxxxxxxxx; Chris Paterson <Chris.Paterson2@xxxxxxxxxxx>;
> > Prabhakar Mahadev Lad <prabhakar.mahadev-lad.rj@xxxxxxxxxxxxxx>;
> > linux-renesas- soc@xxxxxxxxxxxxxxx
> > Subject: Re: [PATCH v10 4/5] counter: Add Renesas RZ/G2L MTU3a counter
> > driver
> >
> > 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?
> 
> OK will use the helper function " rz_mtu3_lock_if_counter_is_valid"
> 
> +static int rz_mtu3_lock_if_counter_is_valid(struct counter_device *counter,
> +                                           struct rz_mtu3_channel *const
> ch,
> +                                           struct rz_mtu3_cnt *const priv,
> +                                           int id) {
> +       if (ch->is_busy && !priv->count_is_enabled[id])
> +               return -EINVAL;
> +
> +       if (rz_mtu3_is_counter_invalid(counter, id))
> +               return -EBUSY;
> +
> +       mutex_lock(&priv->lock);
> +
> +       return 0;
> +}

It will be updated as,

static int rz_mtu3_lock_if_counter_is_valid(struct counter_device *counter,
					    struct rz_mtu3_channel *const ch,
					    struct rz_mtu3_cnt *const priv,
					    int id)
{
	mutex_lock(&priv->lock);

	if (ch->is_busy && !priv->count_is_enabled[id]) {
		mutex_unlock(&priv->lock);
		return -EINVAL;
	}

	if (rz_mtu3_is_counter_invalid(counter, id)) {
		mutex_unlock(&priv->lock);
		return -EBUSY;
	}

	return 0;
}

Cheers,
Biju




[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