On Wed, Dec 14, 2022 at 10:31:35AM +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>
Hello Biju,
Do you need to take the ch->lock before checking ch->is_busy to ensure
it does not change?
Regardless, I have some race comments below.
> +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);
> + u8 timer_mode;
> +
> + if (ch->is_busy && !priv->count_is_enabled[count->id])
> + return -EINVAL;
The priv->lock must be taken because count_is_enabled could change
after it's checked here.
However, you'll need to spin up a helper function because you're
currently calling rz_mtu3_count_function_read() in
rz_mtu3_action_read(). So move the implementation of this function to a
new helper function and call that here with the appropriate locks.
> +static int rz_mtu3_count_direction_read(struct counter_device *counter,
> + struct counter_count *count,
> + enum counter_count_direction *direction)
> +{
> + struct rz_mtu3_channel *const ch = rz_mtu3_get_ch(counter, count->id);
> + struct rz_mtu3_cnt *const priv = counter_priv(counter);
> + u8 tsr;
> +
> + if (ch->is_busy && !priv->count_is_enabled[count->id])
> + return -EINVAL;
This needs to be locked for the same reason as above.
William Breathitt Gray
Attachment:
signature.asc
Description: PGP signature
