On 26/03/2025 at 21:19, Biju Das wrote:
> All existing SoCs support an external clock, but RZ/G3E has only internal
> clocks. Add only_internal_clks to struct rcar_canfd_hw_info to handle this
> difference.
>
> Reviewed-by: Geert Uytterhoeven <geert+renesas@xxxxxxxxx>
> Signed-off-by: Biju Das <biju.das.jz@xxxxxxxxxxxxxx>
A few nits but:
Reviewed-by: Vincent Mailhol <mailhol.vincent@xxxxxxxxxx>
> ---
> v6->v7:
> * No change.
> v5->v6:
> * No change.
> v4->v5:
> * Collected tag.
> * Improved commit description by "All SoCs supports extenal clock"->
> "All existing SoCs support an external clock".
> v3->v4:
> * No change.
> v2->v3:
> * No change
> v1->v2:
> * No change.
> ---
> drivers/net/can/rcar/rcar_canfd.c | 3 ++-
> 1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/net/can/rcar/rcar_canfd.c b/drivers/net/can/rcar/rcar_canfd.c
> index 20e591421cc6..7ad27087a176 100644
> --- a/drivers/net/can/rcar/rcar_canfd.c
> +++ b/drivers/net/can/rcar/rcar_canfd.c
> @@ -546,6 +546,7 @@ struct rcar_canfd_hw_info {
> unsigned multi_channel_irqs:1; /* Has multiple channel irqs */
> unsigned ch_interface_mode:1; /* Has channel interface mode */
> unsigned shared_can_regs:1; /* Has shared classical can registers */
> + unsigned only_internal_clks:1; /* Has only internal clocks */
> };
By default the fields will be set to false if omitted. I think it is a
bit more readable if you still set them explicitly to zero.
> /* Channel priv data */
> @@ -2045,7 +2046,7 @@ static int rcar_canfd_probe(struct platform_device *pdev)
> fcan_freq = clk_get_rate(gpriv->can_clk) / info->postdiv;
> } else {
> fcan_freq = clk_get_rate(gpriv->can_clk);
> - gpriv->extclk = true;
> + gpriv->extclk = !gpriv->info->only_internal_clks;
Nitpick: if at the end, what matters in the extclk boolean, you may
directly name your field:
rcar_canfd_hw_info->has_external_clks
and no need for the negation above.
> }
>
> addr = devm_platform_ioremap_resource(pdev, 0);
Yours sincerely,
Vincent Mailhol
