Re: [PATCH v2 1/3] clk: renesas: rzg2l: Add support for watchdog reset selection

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hi Biju,

On Thu, Nov 18, 2021 at 5:01 PM Biju Das <biju.das.jz@xxxxxxxxxxxxxx> wrote:
> > Subject: Re: [PATCH v2 1/3] clk: renesas: rzg2l: Add support for watchdog
> > reset selection
> > On Wed, Nov 17, 2021 at 9:21 AM Biju Das <biju.das.jz@xxxxxxxxxxxxxx>
> > wrote:
> > > On the, next version I am planning to introduce the below code for
> > > Reset selection based on device availability, instead of selecting all
> > > the channels. Is it the right way to do ? please let me know.
> > >
> > > node = of_find_node_by_name (NULL, NULL, "watchdog@12800800"); if
> > > (node && of_device_is_available(node) {
> > >         // set reset selection for that channel
> > >         of_node_put(node);
> > > }
> > >
> > > node = of_find_node_by_name (NULL, NULL, "watchdog@12800c00"); if
> > > (node && of_device_is_available(node) {
> > >         // set reset selection for that channel
> > >         of_node_put(node);
> > > }
> > >
> > > node = of_find_node_by_name (NULL, NULL, "watchdog@12800400"); if
> > > (node && of_device_is_available(node) {
> > >         // set reset selection for that channel
> > >         of_node_put(node);
> > > }
> >
> > Matching on node names is very fragile.
>
> Agreed.
>
>   And what if the watchdog node is
> > enabled in DT, but the watchdog driver is not available?
> We will just configure, but since there is no watch driver available.
> I guess nothing will happen.
>
> > Moreover, this looks like it should not be controlled from the clock
> > driver, but from the watchdog driver instead.
>
> I have referred configure option from reset driver for R-Car, where WDT is configured
> in reset block as similar register is located in reset block rather the watchdog driver.
>
> May be I should not use Matching on node names, rather use bitmask of bits as you suggested.
>
> Please share your views.

On R-Car Gen2 and RZ/G1 SoCs, this is indeed configured by the
rcar-rst driver, because the support was added later. Initial R-Car
Gen2 revisions had hardware quirks preventing proper operation.
On R-Car Gen3 and other RZ/G2 SoCs, this is configured by the boot
loader, and it would be great if new SoCs (R-Car V3U, R-Car S4-8,
and RZ/G2L) would handle this the same way.

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@xxxxxxxxxxxxxx

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds



[Index of Archives]     [Linux Samsung SOC]     [Linux Wireless]     [Linux Kernel]     [ATH6KL]     [Linux Bluetooth]     [Linux Netdev]     [Kernel Newbies]     [IDE]     [Security]     [Git]     [Netfilter]     [Bugtraq]     [Yosemite News]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux ATA RAID]     [Samba]     [Device Mapper]

  Powered by Linux