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 William Breathitt Gray,

> Subject: Re: [PATCH v10 4/5] counter: Add Renesas RZ/G2L MTU3a counter
> driver
> 
> On Fri, Dec 16, 2022 at 08:50:27PM +0000, Biju Das 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>
> 
> Hi Biju,
> 
> You're missing an entry for this driver in the MAINTAINERS file so please
> add one. The code for this version looks good so you're welcome to add my Rb
> line.

OK.

> 
> Reviewed-by: William Breathitt Gray <william.gray@xxxxxxxxxx>
> 
> I do have a minor suggestion below.
> 
> > +static struct counter_count rz_mtu3_counts[] = {
> > +	{
> > +		.id = RZ_MTU3_16_BIT_MTU1_CH,
> > +		.name = "Channel 1 Count",
> > +		.functions_list = rz_mtu3_count_functions,
> > +		.num_functions = ARRAY_SIZE(rz_mtu3_count_functions),
> > +		.synapses = rz_mtu3_mtu1_count_synapses,
> > +		.num_synapses = ARRAY_SIZE(rz_mtu3_mtu1_count_synapses),
> > +		.ext = rz_mtu3_count_ext,
> > +		.num_ext = ARRAY_SIZE(rz_mtu3_count_ext),
> > +	},
> > +	{
> > +		.id = RZ_MTU3_16_BIT_MTU2_CH,
> > +		.name = "Channel 2 Count",
> > +		.functions_list = rz_mtu3_count_functions,
> > +		.num_functions = ARRAY_SIZE(rz_mtu3_count_functions),
> > +		.synapses = rz_mtu3_mtu2_count_synapses,
> > +		.num_synapses = ARRAY_SIZE(rz_mtu3_mtu2_count_synapses),
> > +		.ext = rz_mtu3_count_ext,
> > +		.num_ext = ARRAY_SIZE(rz_mtu3_count_ext),
> > +	},
> > +	{
> > +		.id = RZ_MTU3_32_BIT_CH,
> > +		.name = "Channel 1 and 2 (combined) Count",
> 
> These channels are actually cascaded, so replacing "combined" with
> "cascaded" here would be better.

OK, will fix this in next version.

Cheers,
Biju




[Index of Archives]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Input]     [Linux Kernel]     [Linux SCSI]     [X.org]

  Powered by Linux