Re: [PATCH v6 33/33] staging: mt7621-pci: replace 'mdelay()' with 'msleep()'

[Sergio Paracuellos <sergio.paracuellos@gmail.com>]

On Mon, Nov 05, 2018 at 09:09:39AM +0100, Frans Klaver wrote:
> On Sun, Nov 4, 2018 at 11:51 AM Sergio Paracuellos
> <sergio.paracuellos@gmail.com> wrote:
> >
> > Function 'mt7621_pcie_init_ports' is never called in atomic context.
> > It calls mdelay() to busily wait, which is not necessary. mdelay()
> > can be replaced with msleep().
> >
> > Signed-off-by: Sergio Paracuellos <sergio.paracuellos@gmail.com>
> > ---
> >  drivers/staging/mt7621-pci/pci-mt7621.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/drivers/staging/mt7621-pci/pci-mt7621.c b/drivers/staging/mt7621-pci/pci-mt7621.c
> > index fb9aa6b..14cec23 100644
> > --- a/drivers/staging/mt7621-pci/pci-mt7621.c
> > +++ b/drivers/staging/mt7621-pci/pci-mt7621.c
> > @@ -632,7 +632,7 @@ static void mt7621_pcie_init_ports(struct mt7621_pcie *pcie)
> >         rt_sysc_m32(PCIE_CLK_GEN_EN, PCIE_CLK_GEN_DIS, RALINK_PCIE_CLK_GEN);
> >         rt_sysc_m32(PCIE_CLK_GEN1_DIS, PCIE_CLK_GEN1_EN, RALINK_PCIE_CLK_GEN1);
> >         rt_sysc_m32(PCIE_CLK_GEN_DIS, PCIE_CLK_GEN_EN, RALINK_PCIE_CLK_GEN);
> > -       mdelay(50);
> > +       msleep(50);
> >         rt_sysc_m32(RALINK_PCIE_RST, 0, RALINK_RSTCTRL);

Hi Frans,
> 
> From a software point of view it makes sense to remove the busy-wait.
> From a hardware perspective it may still make sense to keep it. Can
> you guarantee that the PCIE_RST is done at the correct time. I haven't
> looked thoroughly at the datasheet yet, but I can imagine the level
> has to shift within a certain time frame? Or doesn't that make too
> much of a difference? Perhaps mention that in your commit message?

Thanks for the feedback. I agree with you that from a hardware point of
view keep this could make sense. The fact is that after reading the 
datasheet for me is not clear at all that in this concrete case it make
sense. I cannot also test this because I don't have the hardware to be able
to do (all the patches, as I said are only compile-tested). This is also
a reason to keep this patch as the last in this series to just skip it if
this fails when is tested.

Best regards,
    Sergio Paracuellos
_______________________________________________
devel mailing list
devel@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel