| Message ID | 1512585925-4251-1-git-send-email-ykaneko0929@gmail.com (mailing list archive) |
|---|---|
| State | Superseded |
| Delegated to: | Geert Uytterhoeven |
| Headers | show |
Hi Kaneko-san, On Wed, Dec 6, 2017 at 7:45 PM, Yoshihiro Kaneko <ykaneko0929@gmail.com> wrote: > From: Khiem Nguyen <khiem.nguyen.xt@rvc.renesas.com> > > Because power of Salvator-X board is cut off in suspend, > it needs to reset SATA PHY state in resume. > Otherwise, SATA partition could not be accessed anymore. Thanks for your patch! So this is needed on R-Car Gen3 only. > --- a/drivers/ata/sata_rcar.c > +++ b/drivers/ata/sata_rcar.c > @@ -977,11 +977,43 @@ static int sata_rcar_resume(struct device *dev) > struct sata_rcar_priv *priv = host->private_data; > void __iomem *base = priv->base; > int ret; > + u32 val; > > ret = clk_prepare_enable(priv->clk); > if (ret) > return ret; > > + /* Re-use from sata_rcar_init_controller() */ > + /* reset and setup phy */ > + switch (priv->type) { > + case RCAR_GEN1_SATA: > + sata_rcar_gen1_phy_init(priv); Hence why do this (and the below) on R-Car Gen1, too? > + break; > + case RCAR_GEN2_SATA: > + sata_rcar_gen2_phy_init(priv); And on both R-Car Gen2 and Gen3 (currently Gen3 is treated like Gen2 everywhere in the driver)? What about introducing RCAR_GEN3_SATA, and doing the reinit on R-Car Gen3 only? > + break; > + default: > + dev_warn(host->dev, "SATA phy is not initialized\n"); > + break; > + } [...] Gr{oetje,eeting}s, Geert -- Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org 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
On Wed, Dec 06, 2017 at 07:56:09PM +0100, Geert Uytterhoeven wrote: > Hi Kaneko-san, > > On Wed, Dec 6, 2017 at 7:45 PM, Yoshihiro Kaneko <ykaneko0929@gmail.com> wrote: > > From: Khiem Nguyen <khiem.nguyen.xt@rvc.renesas.com> > > > > Because power of Salvator-X board is cut off in suspend, > > it needs to reset SATA PHY state in resume. > > Otherwise, SATA partition could not be accessed anymore. > > Thanks for your patch! > > So this is needed on R-Car Gen3 only. > > > --- a/drivers/ata/sata_rcar.c > > +++ b/drivers/ata/sata_rcar.c > > @@ -977,11 +977,43 @@ static int sata_rcar_resume(struct device *dev) > > struct sata_rcar_priv *priv = host->private_data; > > void __iomem *base = priv->base; > > int ret; > > + u32 val; > > > > ret = clk_prepare_enable(priv->clk); > > if (ret) > > return ret; > > > > + /* Re-use from sata_rcar_init_controller() */ > > + /* reset and setup phy */ > > + switch (priv->type) { > > + case RCAR_GEN1_SATA: > > + sata_rcar_gen1_phy_init(priv); > > Hence why do this (and the below) on R-Car Gen1, too? > > > + break; > > + case RCAR_GEN2_SATA: > > + sata_rcar_gen2_phy_init(priv); > > And on both R-Car Gen2 and Gen3 (currently Gen3 is treated like Gen2 > everywhere in the driver)? > What about introducing RCAR_GEN3_SATA, and doing the reinit on R-Car Gen3 > only? Agreed, that seems to make sense. Kaneko-san, could you look into implementing this? > > > + break; > > + default: > > + dev_warn(host->dev, "SATA phy is not initialized\n"); > > + break; > > + } > > [...] > > Gr{oetje,eeting}s, > > Geert > > -- > Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org > > 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 >
Hi Simon-san, Geert-san, 2017-12-19 17:26 GMT+09:00 Simon Horman <horms@verge.net.au>: > On Wed, Dec 06, 2017 at 07:56:09PM +0100, Geert Uytterhoeven wrote: >> Hi Kaneko-san, >> >> On Wed, Dec 6, 2017 at 7:45 PM, Yoshihiro Kaneko <ykaneko0929@gmail.com> wrote: >> > From: Khiem Nguyen <khiem.nguyen.xt@rvc.renesas.com> >> > >> > Because power of Salvator-X board is cut off in suspend, >> > it needs to reset SATA PHY state in resume. >> > Otherwise, SATA partition could not be accessed anymore. >> >> Thanks for your patch! >> >> So this is needed on R-Car Gen3 only. >> >> > --- a/drivers/ata/sata_rcar.c >> > +++ b/drivers/ata/sata_rcar.c >> > @@ -977,11 +977,43 @@ static int sata_rcar_resume(struct device *dev) >> > struct sata_rcar_priv *priv = host->private_data; >> > void __iomem *base = priv->base; >> > int ret; >> > + u32 val; >> > >> > ret = clk_prepare_enable(priv->clk); >> > if (ret) >> > return ret; >> > >> > + /* Re-use from sata_rcar_init_controller() */ >> > + /* reset and setup phy */ >> > + switch (priv->type) { >> > + case RCAR_GEN1_SATA: >> > + sata_rcar_gen1_phy_init(priv); >> >> Hence why do this (and the below) on R-Car Gen1, too? >> >> > + break; >> > + case RCAR_GEN2_SATA: >> > + sata_rcar_gen2_phy_init(priv); >> >> And on both R-Car Gen2 and Gen3 (currently Gen3 is treated like Gen2 >> everywhere in the driver)? >> What about introducing RCAR_GEN3_SATA, and doing the reinit on R-Car Gen3 >> only? > > Agreed, that seems to make sense. > Kaneko-san, could you look into implementing this? Yes, I will do it. Kaneko > >> >> > + break; >> > + default: >> > + dev_warn(host->dev, "SATA phy is not initialized\n"); >> > + break; >> > + } >> >> [...] >> >> Gr{oetje,eeting}s, >> >> Geert >> >> -- >> Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org >> >> 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 >>
Hello! On 12/6/2017 9:45 PM, Yoshihiro Kaneko wrote: > From: Khiem Nguyen <khiem.nguyen.xt@rvc.renesas.com> Please use "sata_rcar: " prefix for the subject. > Because power of Salvator-X board is cut off in suspend, > it needs to reset SATA PHY state in resume. > Otherwise, SATA partition could not be accessed anymore. > > Signed-off-by: Khiem Nguyen <khiem.nguyen.xt@rvc.renesas.com> > Signed-off-by: Hien Dang <hien.dang.eb@rvc.renesas.com> > Signed-off-by: Yoshihiro Kaneko <ykaneko0929@gmail.com> [...] MBR, Sergei
Hello! On 12/6/2017 9:45 PM, Yoshihiro Kaneko wrote: > From: Khiem Nguyen <khiem.nguyen.xt@rvc.renesas.com> > > Because power of Salvator-X board is cut off in suspend, > it needs to reset SATA PHY state in resume. > Otherwise, SATA partition could not be accessed anymore. > > Signed-off-by: Khiem Nguyen <khiem.nguyen.xt@rvc.renesas.com> > Signed-off-by: Hien Dang <hien.dang.eb@rvc.renesas.com> > Signed-off-by: Yoshihiro Kaneko <ykaneko0929@gmail.com> > --- > > This patch is based on the for-next branch of libata tree. > > drivers/ata/sata_rcar.c | 32 ++++++++++++++++++++++++++++++++ > 1 file changed, 32 insertions(+) > > diff --git a/drivers/ata/sata_rcar.c b/drivers/ata/sata_rcar.c > index 80ee2f2..aba6121 100644 > --- a/drivers/ata/sata_rcar.c > +++ b/drivers/ata/sata_rcar.c > @@ -977,11 +977,43 @@ static int sata_rcar_resume(struct device *dev) > struct sata_rcar_priv *priv = host->private_data; > void __iomem *base = priv->base; > int ret; > + u32 val; > > ret = clk_prepare_enable(priv->clk); > if (ret) > return ret; > > + /* Re-use from sata_rcar_init_controller() */ Why not just call it? Or at least factor out a common part? [...] MBR, Sergei
diff --git a/drivers/ata/sata_rcar.c b/drivers/ata/sata_rcar.c index 80ee2f2..aba6121 100644 --- a/drivers/ata/sata_rcar.c +++ b/drivers/ata/sata_rcar.c @@ -977,11 +977,43 @@ static int sata_rcar_resume(struct device *dev) struct sata_rcar_priv *priv = host->private_data; void __iomem *base = priv->base; int ret; + u32 val; ret = clk_prepare_enable(priv->clk); if (ret) return ret; + /* Re-use from sata_rcar_init_controller() */ + /* reset and setup phy */ + switch (priv->type) { + case RCAR_GEN1_SATA: + sata_rcar_gen1_phy_init(priv); + break; + case RCAR_GEN2_SATA: + sata_rcar_gen2_phy_init(priv); + break; + default: + dev_warn(host->dev, "SATA phy is not initialized\n"); + break; + } + + /* SATA-IP reset state */ + val = ioread32(base + ATAPI_CONTROL1_REG); + val |= ATAPI_CONTROL1_RESET; + iowrite32(val, base + ATAPI_CONTROL1_REG); + + /* ISM mode, PRD mode, DTEND flag at bit 0 */ + val = ioread32(base + ATAPI_CONTROL1_REG); + val |= ATAPI_CONTROL1_ISM; + val |= ATAPI_CONTROL1_DESE; + val |= ATAPI_CONTROL1_DTA32M; + iowrite32(val, base + ATAPI_CONTROL1_REG); + + /* Release the SATA-IP from the reset state */ + val = ioread32(base + ATAPI_CONTROL1_REG); + val &= ~ATAPI_CONTROL1_RESET; + iowrite32(val, base + ATAPI_CONTROL1_REG); + /* ack and mask */ iowrite32(0, base + SATAINTSTAT_REG); iowrite32(0x7ff, base + SATAINTMASK_REG);