On Tue, Nov 15, 2022 at 10:38:32AM +0000, Biju Das wrote:
> Hi William Breathitt Gray,
>
> Thanks for the feedback.
>
> > -----Original Message-----
> > From: William Breathitt Gray <william.gray@xxxxxxxxxx>
> > Sent: 15 November 2022 04:53
> > To: Biju Das <biju.das.jz@xxxxxxxxxxxxxx>
> > Cc: linux-iio@xxxxxxxxxxxxxxx; Geert Uytterhoeven
> > <geert+renesas@xxxxxxxxx>; Chris Paterson
> > <Chris.Paterson2@xxxxxxxxxxx>; Prabhakar Mahadev Lad
> > <prabhakar.mahadev-lad.rj@xxxxxxxxxxxxxx>; linux-renesas-
> > soc@xxxxxxxxxxxxxxx
> > Subject: Re: [PATCH v6 4/5] counter: Add Renesas RZ/G2L MTU3a counter
> > driver
> >
> > On Mon, Nov 14, 2022 at 05:52:11PM +0000, Biju Das wrote:
> > > > > > +static int rz_mtu3_initialize_counter(struct counter_device
> > > > > > +*counter, int id) {
> > > > > > + struct rz_mtu3_cnt *const priv = counter_priv(counter);
> > > > > > + struct rz_mtu3_channel *ch1 = priv->ch;
> > > > > > + struct rz_mtu3_channel *ch2 = ch1 + 1;
> > > > >
> > > > > No need to complicate this, just use priv->ch[0], priv->ch[1],
> > and
> > > > > priv->ch[id]. Same advice applies to the other functions as
> > well.
> > > >
> > > > I get below error when I use array susbscripts. "*ch1 = priv-
> > >ch[0];"
> > >
> > > > drivers/counter/rz-mtu3-cnt.c:291:32: error: incompatible types
> > when
> > > > initialising type 'struct rz_mtu3_channel *' using type 'struct
> > > > rz_mtu3_channel'
> > > > 291 | struct rz_mtu3_channel *ch1 = priv->ch[0];
> > > >
> > >
> > > I could use "*ch1 = &priv->ch[0];" please let me know is it ok?
> > >
> > > Cheers,
> > > Biju
> >
> > Hi Biju,
> >
> > I meant to use the array subscripts inline (e.g. priv-
> > >ch[id].function).
> > However, I can see the benefit of using the ch1 and ch2 local
> > variables, so perhaps something like this would be clearer to read:
> >
> > struct rz_mtu3_chanel *const ch = priv->ch;
> > struct rz_mtu3_chanel *const ch1 = &ch[0];
> > struct rz_mtu3_chanel *const ch2 = &ch[1];
> > ...
> > case RZ_MTU3_16_BIT_MTU1_CH:
> > case RZ_MTU3_16_BIT_MTU2_CH:
> > if (ch[id].function != RZ_MTU3_NORMAL) {
> > ...
>
>
> OK, I have added below inline function which simplifies the code
> in each function. Is it ok?
>
> For eg:
>
> +static inline struct rz_mtu3_channel *
> +rz_mtu3_get_ch(struct counter_device *counter, int id)
> +{
> + struct rz_mtu3_cnt *const priv = counter_priv(counter);
> + const size_t ch_id = RZ_MTU3_GET_HW_CH(id);
> +
> + return &priv->ch[ch_id];
> +}
>
>
> @@ -154,11 +163,10 @@ static int rz_mtu3_count_function_read(struct counter_device *counter,
> struct counter_count *count,
> enum counter_function *function)
> {
> - struct rz_mtu3_cnt *const priv = counter_priv(counter);
> - const size_t ch_id = RZ_MTU3_GET_HW_CH(count->id);
> + struct rz_mtu3_channel *const ch = rz_mtu3_get_ch(counter, count->id);
>
>
> Cheers,
> Biju
Sure, I think that function will be okay to use.
William Breathitt Gray
Attachment:
signature.asc
Description: PGP signature
