| Message ID | 1516113552-27701-1-git-send-email-ykaneko0929@gmail.com (mailing list archive) |
|---|---|
| State | Superseded |
| Delegated to: | Geert Uytterhoeven |
| Headers | show |
Hello! On 01/16/2018 05:39 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> > [reinit phy in sata_rcar_resume() function on R-Car Gen3 only] > [fixed the prefix for the subject] > Signed-off-by: Yoshihiro Kaneko <ykaneko0929@gmail.com> > --- > > This patch is based on the for-next branch of libata tree. > > v2 [Yoshihiro Kaneko] > * reinit phy on R-Car Gen3 only as suggested by Geert Uytterhoeven > * use "sata_rcar" prefix for the subject as suggested by Sergei Shtylyov > drivers/ata/sata_rcar.c | 38 ++++++++++++++++++++++++++++++++++++-- > 1 file changed, 36 insertions(+), 2 deletions(-) > > diff --git a/drivers/ata/sata_rcar.c b/drivers/ata/sata_rcar.c > index 80ee2f2..4adc0d6 100644 > --- a/drivers/ata/sata_rcar.c > +++ b/drivers/ata/sata_rcar.c [...] > @@ -977,11 +979,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; > > + /* reinit phy on R-Car Gen3 only */ > + switch (priv->type) { > + case RCAR_GEN1_SATA: > + case RCAR_GEN2_SATA: > + case RCAR_R8A7790_ES1_SATA: > + break; > + case RCAR_GEN3_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); > + As it stands, we could surely do the same with a much shorter code: if (priv->type == RCAR_GEN3_SATA) { sata_rcar_init_controller(host); } else { /* ack and mask */ iowrite32(0, base + SATAINTSTAT_REG); iowrite32(0x7ff, base + SATAINTMASK_REG); } But do we really need this reset/init sequence on gen1/2 chips? MBR, Sergei
On 01/16/2018 05:39 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> > [reinit phy in sata_rcar_resume() function on R-Car Gen3 only] > [fixed the prefix for the subject] > Signed-off-by: Yoshihiro Kaneko <ykaneko0929@gmail.com> > --- > > This patch is based on the for-next branch of libata tree. > > v2 [Yoshihiro Kaneko] > * reinit phy on R-Car Gen3 only as suggested by Geert Uytterhoeven > * use "sata_rcar" prefix for the subject as suggested by Sergei Shtylyov And I don't think you need the redundant "sata: " prefix either... MBR, Sergei
On 01/16/2018 07:25 PM, Sergei Shtylyov 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> >> [reinit phy in sata_rcar_resume() function on R-Car Gen3 only] >> [fixed the prefix for the subject] >> Signed-off-by: Yoshihiro Kaneko <ykaneko0929@gmail.com> >> --- >> >> This patch is based on the for-next branch of libata tree. >> >> v2 [Yoshihiro Kaneko] >> * reinit phy on R-Car Gen3 only as suggested by Geert Uytterhoeven >> * use "sata_rcar" prefix for the subject as suggested by Sergei Shtylyov >> drivers/ata/sata_rcar.c | 38 ++++++++++++++++++++++++++++++++++++-- >> 1 file changed, 36 insertions(+), 2 deletions(-) >> >> diff --git a/drivers/ata/sata_rcar.c b/drivers/ata/sata_rcar.c >> index 80ee2f2..4adc0d6 100644 >> --- a/drivers/ata/sata_rcar.c >> +++ b/drivers/ata/sata_rcar.c > [...] >> @@ -977,11 +979,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; >> + /* reinit phy on R-Car Gen3 only */ >> + switch (priv->type) { >> + case RCAR_GEN1_SATA: >> + case RCAR_GEN2_SATA: >> + case RCAR_R8A7790_ES1_SATA: >> + break; >> + case RCAR_GEN3_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); >> + > > As it stands, we could surely do the same with a much shorter code: > > if (priv->type == RCAR_GEN3_SATA) { > sata_rcar_init_controller(host); > } else { > /* ack and mask */ > iowrite32(0, base + SATAINTSTAT_REG); > iowrite32(0x7ff, base + SATAINTMASK_REG); > } > > But do we really need this reset/init sequence on gen1/2 chips? Oops, the above suggestion is what I think is a correct approach, it can't just replace your code... MBR, Sergei
diff --git a/drivers/ata/sata_rcar.c b/drivers/ata/sata_rcar.c index 80ee2f2..4adc0d6 100644 --- a/drivers/ata/sata_rcar.c +++ b/drivers/ata/sata_rcar.c @@ -146,6 +146,7 @@ enum sata_rcar_type { RCAR_GEN1_SATA, RCAR_GEN2_SATA, + RCAR_GEN3_SATA, RCAR_R8A7790_ES1_SATA, }; @@ -796,6 +797,7 @@ static void sata_rcar_init_controller(struct ata_host *host) sata_rcar_gen1_phy_init(priv); break; case RCAR_GEN2_SATA: + case RCAR_GEN3_SATA: case RCAR_R8A7790_ES1_SATA: sata_rcar_gen2_phy_init(priv); break; @@ -856,7 +858,7 @@ static void sata_rcar_init_controller(struct ata_host *host) }, { .compatible = "renesas,sata-r8a7795", - .data = (void *)RCAR_GEN2_SATA + .data = (void *)RCAR_GEN3_SATA }, { .compatible = "renesas,rcar-gen2-sata", @@ -864,7 +866,7 @@ static void sata_rcar_init_controller(struct ata_host *host) }, { .compatible = "renesas,rcar-gen3-sata", - .data = (void *)RCAR_GEN2_SATA + .data = (void *)RCAR_GEN3_SATA }, { }, }; @@ -977,11 +979,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; + /* reinit phy on R-Car Gen3 only */ + switch (priv->type) { + case RCAR_GEN1_SATA: + case RCAR_GEN2_SATA: + case RCAR_R8A7790_ES1_SATA: + break; + case RCAR_GEN3_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);