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

[William Breathitt Gray <william.gray@linaro.org>]

[-- Attachment #1: Type: text/plain, Size: 1421 bytes --]

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) {
    ...

William Breathitt Gray

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]