| Message ID | 1381432083-3684-1-git-send-email-valentine.barshak@cogentembedded.com (mailing list archive) |
|---|---|
| State | Awaiting Upstream |
| Headers | show |
[ CCed devicetree@vger.kernel.org as this involves DT compatibility strings ] On Thu, Oct 10, 2013 at 11:08:03PM +0400, Valentine Barshak wrote: > RCAR Gen2 SoC has a different phy which is not compatible with > the older H1/M1 versions. This adds OF/platform device table > and PHY initialization callbacks for H2/M2 (Gen2) SoC. I think it would aid subsequent reading of this patch if you included information about which SoC is H1, M1, H2 and M2 in the change log. For the benefit of others they are as follows: Gen 1: H1: r8a7779 SoC M1: r8a7778 SoC Gen 2: H2: r8a7790 SoC M2: r8a7791 SoC > PHY initialization method is chosen based on the device id. > Default PHY settings are applied for Gen2 SoC, which should > suit the available Gen2 boards. > > Signed-off-by: Valentine Barshak <valentine.barshak@cogentembedded.com> > --- > drivers/ata/sata_rcar.c | 108 +++++++++++++++++++++++++++++++++++++++++------- > 1 file changed, 94 insertions(+), 14 deletions(-) > > diff --git a/drivers/ata/sata_rcar.c b/drivers/ata/sata_rcar.c > index c2d95e9..45af29f 100644 > --- a/drivers/ata/sata_rcar.c > +++ b/drivers/ata/sata_rcar.c > @@ -15,6 +15,7 @@ > #include <linux/module.h> > #include <linux/ata.h> > #include <linux/libata.h> > +#include <linux/of_device.h> > #include <linux/platform_device.h> > #include <linux/clk.h> > #include <linux/err.h> > @@ -118,17 +119,42 @@ > #define SATAPCTLR3_REG 0x5A > #define SATAPCTLR4_REG 0x60 > > +/* Gen2 Physical Layer Control Registers */ > +#define RCAR_GEN2_PHY_CTL1_REG 0x1704 > +#define RCAR_GEN2_PHY_CTL1 0x34180002 > +#define RCAR_GEN2_PHY_CTL1_SS 0xC180 /* Spread Spectrum */ > + > +#define RCAR_GEN2_PHY_CTL2_REG 0x170C > +#define RCAR_GEN2_PHY_CTL2 0x00002303 > + > +#define RCAR_GEN2_PHY_CTL3_REG 0x171C > +#define RCAR_GEN2_PHY_CTL3 0x000B0194 > + > +#define RCAR_GEN2_PHY_CTL4_REG 0x1724 > +#define RCAR_GEN2_PHY_CTL4 0x00030994 > + > +#define RCAR_GEN2_PHY_CTL5_REG 0x1740 > +#define RCAR_GEN2_PHY_CTL5 0x03004001 > +#define RCAR_GEN2_PHY_CTL5_DC BIT(1) /* DC connection */ > +#define RCAR_GEN2_PHY_CTL5_TR BIT(2) /* Termination Resistor */ > + > /* Descriptor table word 0 bit (when DTA32M = 1) */ > #define SATA_RCAR_DTEND BIT(0) > > #define SATA_RCAR_DMA_BOUNDARY 0x1FFFFFFEUL > > +enum sata_rcar_type { > + RCAR_SATA, > + RCAR_GEN2_SATA, > +}; > + > struct sata_rcar_priv { > void __iomem *base; > struct clk *clk; > + enum sata_rcar_type type; > }; > > -static void sata_rcar_phy_initialize(struct sata_rcar_priv *priv) > +static void sata_rcar_phy_preinit(struct sata_rcar_priv *priv) > { > void __iomem *base = priv->base; > > @@ -170,6 +196,29 @@ static void sata_rcar_phy_write(struct sata_rcar_priv *priv, u16 reg, u32 val, > iowrite32(0, base + SATAPHYADDR_REG); > } > > +static void sata_rcar_phy_init(struct sata_rcar_priv *priv) > +{ > + sata_rcar_phy_preinit(priv); > + sata_rcar_phy_write(priv, SATAPCTLR1_REG, 0x00200188, 0); > + sata_rcar_phy_write(priv, SATAPCTLR1_REG, 0x00200188, 1); > + sata_rcar_phy_write(priv, SATAPCTLR3_REG, 0x0000A061, 0); > + sata_rcar_phy_write(priv, SATAPCTLR2_REG, 0x20000000, 0); > + sata_rcar_phy_write(priv, SATAPCTLR2_REG, 0x20000000, 1); > + sata_rcar_phy_write(priv, SATAPCTLR4_REG, 0x28E80000, 0); > +} > + > +static void sata_rcar_gen2_phy_init(struct sata_rcar_priv *priv) > +{ > + void __iomem *base = priv->base; > + > + iowrite32(RCAR_GEN2_PHY_CTL1, base + RCAR_GEN2_PHY_CTL1_REG); > + iowrite32(RCAR_GEN2_PHY_CTL2, base + RCAR_GEN2_PHY_CTL2_REG); > + iowrite32(RCAR_GEN2_PHY_CTL3, base + RCAR_GEN2_PHY_CTL3_REG); > + iowrite32(RCAR_GEN2_PHY_CTL4, base + RCAR_GEN2_PHY_CTL4_REG); > + iowrite32(RCAR_GEN2_PHY_CTL5 | RCAR_GEN2_PHY_CTL5_DC | > + RCAR_GEN2_PHY_CTL5_TR, base + RCAR_GEN2_PHY_CTL5_REG); > +} > + > static void sata_rcar_freeze(struct ata_port *ap) > { > struct sata_rcar_priv *priv = ap->host->private_data; > @@ -738,13 +787,17 @@ static void sata_rcar_init_controller(struct ata_host *host) > u32 val; > > /* reset and setup phy */ > - sata_rcar_phy_initialize(priv); > - sata_rcar_phy_write(priv, SATAPCTLR1_REG, 0x00200188, 0); > - sata_rcar_phy_write(priv, SATAPCTLR1_REG, 0x00200188, 1); > - sata_rcar_phy_write(priv, SATAPCTLR3_REG, 0x0000A061, 0); > - sata_rcar_phy_write(priv, SATAPCTLR2_REG, 0x20000000, 0); > - sata_rcar_phy_write(priv, SATAPCTLR2_REG, 0x20000000, 1); > - sata_rcar_phy_write(priv, SATAPCTLR4_REG, 0x28E80000, 0); > + switch (priv->type) { > + case RCAR_GEN2_SATA: > + sata_rcar_gen2_phy_init(priv); > + break; > + case RCAR_SATA: > + sata_rcar_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); > @@ -770,8 +823,34 @@ static void sata_rcar_init_controller(struct ata_host *host) > iowrite32(ATAPI_INT_ENABLE_SATAINT, base + ATAPI_INT_ENABLE_REG); > } > > +static struct of_device_id sata_rcar_match[] = { > + { > + .compatible = "renesas,rcar-sata", > + .data = (void *)RCAR_SATA > + }, > + { > + .compatible = "renesas,sata-r8a7790", > + .data = (void *)RCAR_GEN2_SATA > + }, > + { > + .compatible = "renesas,sata-r8a7791", > + .data = (void *)RCAR_GEN2_SATA > + }, > + {}, > +}; > +MODULE_DEVICE_TABLE(of, sata_rcar_match); > + > +static const struct platform_device_id sata_rcar_id_table[] = { > + { "sata_rcar", RCAR_SATA }, > + { "sata-r8a7790", RCAR_GEN2_SATA }, > + { "sata-r8a7791", RCAR_GEN2_SATA }, > + { }, > +}; > +MODULE_DEVICE_TABLE(platform, sata_rcat_id_table); > + I think it would be better to add sata-r8a7779 and sata-r8a7778 to handle the GEN1 hardware and deprecate rcar-sata. Less importantly I think it would be better to name RCAR_SATA as RCAR_GEN1_SATA. > static int sata_rcar_probe(struct platform_device *pdev) > { > + const struct of_device_id *of_id; > struct ata_host *host; > struct sata_rcar_priv *priv; > struct resource *mem; > @@ -787,6 +866,12 @@ static int sata_rcar_probe(struct platform_device *pdev) > if (!priv) > return -ENOMEM; > > + of_id = of_match_device(sata_rcar_match, &pdev->dev); > + if (of_id) > + priv->type = (enum sata_rcar_type)of_id->data; > + else > + priv->type = platform_get_device_id(pdev)->driver_data; > + > priv->clk = devm_clk_get(&pdev->dev, NULL); > if (IS_ERR(priv->clk)) { > dev_err(&pdev->dev, "failed to get access to sata clock\n"); > @@ -892,15 +977,10 @@ static const struct dev_pm_ops sata_rcar_pm_ops = { > }; > #endif > > -static struct of_device_id sata_rcar_match[] = { > - { .compatible = "renesas,rcar-sata", }, > - {}, > -}; > -MODULE_DEVICE_TABLE(of, sata_rcar_match); > - > static struct platform_driver sata_rcar_driver = { > .probe = sata_rcar_probe, > .remove = sata_rcar_remove, > + .id_table = sata_rcar_id_table, > .driver = { > .name = DRV_NAME, > .owner = THIS_MODULE, > -- > 1.8.3.1 > -- To unsubscribe from this list: send the line "unsubscribe linux-sh" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Fri, Oct 11, 2013 at 02:00:35AM +0100, Simon Horman wrote: > [ CCed devicetree@vger.kernel.org as this involves DT compatibility strings ] Cheers! > > On Thu, Oct 10, 2013 at 11:08:03PM +0400, Valentine Barshak wrote: > > RCAR Gen2 SoC has a different phy which is not compatible with > > the older H1/M1 versions. This adds OF/platform device table > > and PHY initialization callbacks for H2/M2 (Gen2) SoC. Is the PHY combined with the rest of the controller, or are they logically separate components in the SoC? I note that the Calxeda Highbank SATA controller driver treats the phy and the controller as separate entities, and describes the way the two are attached (though the driver handles both). Would a similar approach work here? [...] > > +static struct of_device_id sata_rcar_match[] = { > > + { > > + .compatible = "renesas,rcar-sata", > > + .data = (void *)RCAR_SATA > > + }, > > + { > > + .compatible = "renesas,sata-r8a7790", > > + .data = (void *)RCAR_GEN2_SATA > > + }, > > + { > > + .compatible = "renesas,sata-r8a7791", > > + .data = (void *)RCAR_GEN2_SATA > > + }, > > + {}, These bindings will need documentation. A grep of any of these in mainline's Documentation/devicetree shows up nothing (not even the existing "renesas,rcar-sata" string used by the driver. Thanks, Mark. -- To unsubscribe from this list: send the line "unsubscribe linux-sh" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 10/11/2013 05:00 AM, Simon Horman wrote: > [ CCed devicetree@vger.kernel.org as this involves DT compatibility strings ] > > On Thu, Oct 10, 2013 at 11:08:03PM +0400, Valentine Barshak wrote: >> RCAR Gen2 SoC has a different phy which is not compatible with >> the older H1/M1 versions. This adds OF/platform device table >> and PHY initialization callbacks for H2/M2 (Gen2) SoC. > > I think it would aid subsequent reading of this patch > if you included information about which SoC is H1, M1, H2 and M2 > in the change log. > > For the benefit of others they are as follows: > > Gen 1: > H1: r8a7779 SoC > M1: r8a7778 SoC > > Gen 2: > H2: r8a7790 SoC > M2: r8a7791 SoC > OK, thanks. I'll add this to the commit log. >> PHY initialization method is chosen based on the device id. >> Default PHY settings are applied for Gen2 SoC, which should >> suit the available Gen2 boards. >> >> Signed-off-by: Valentine Barshak <valentine.barshak@cogentembedded.com> >> --- >> drivers/ata/sata_rcar.c | 108 +++++++++++++++++++++++++++++++++++++++++------- >> 1 file changed, 94 insertions(+), 14 deletions(-) >> >> diff --git a/drivers/ata/sata_rcar.c b/drivers/ata/sata_rcar.c >> index c2d95e9..45af29f 100644 >> --- a/drivers/ata/sata_rcar.c >> +++ b/drivers/ata/sata_rcar.c >> @@ -15,6 +15,7 @@ >> #include <linux/module.h> >> #include <linux/ata.h> >> #include <linux/libata.h> >> +#include <linux/of_device.h> >> #include <linux/platform_device.h> >> #include <linux/clk.h> >> #include <linux/err.h> >> @@ -118,17 +119,42 @@ >> #define SATAPCTLR3_REG 0x5A >> #define SATAPCTLR4_REG 0x60 >> >> +/* Gen2 Physical Layer Control Registers */ >> +#define RCAR_GEN2_PHY_CTL1_REG 0x1704 >> +#define RCAR_GEN2_PHY_CTL1 0x34180002 >> +#define RCAR_GEN2_PHY_CTL1_SS 0xC180 /* Spread Spectrum */ >> + >> +#define RCAR_GEN2_PHY_CTL2_REG 0x170C >> +#define RCAR_GEN2_PHY_CTL2 0x00002303 >> + >> +#define RCAR_GEN2_PHY_CTL3_REG 0x171C >> +#define RCAR_GEN2_PHY_CTL3 0x000B0194 >> + >> +#define RCAR_GEN2_PHY_CTL4_REG 0x1724 >> +#define RCAR_GEN2_PHY_CTL4 0x00030994 >> + >> +#define RCAR_GEN2_PHY_CTL5_REG 0x1740 >> +#define RCAR_GEN2_PHY_CTL5 0x03004001 >> +#define RCAR_GEN2_PHY_CTL5_DC BIT(1) /* DC connection */ >> +#define RCAR_GEN2_PHY_CTL5_TR BIT(2) /* Termination Resistor */ >> + >> /* Descriptor table word 0 bit (when DTA32M = 1) */ >> #define SATA_RCAR_DTEND BIT(0) >> >> #define SATA_RCAR_DMA_BOUNDARY 0x1FFFFFFEUL >> >> +enum sata_rcar_type { >> + RCAR_SATA, >> + RCAR_GEN2_SATA, >> +}; >> + >> struct sata_rcar_priv { >> void __iomem *base; >> struct clk *clk; >> + enum sata_rcar_type type; >> }; >> >> -static void sata_rcar_phy_initialize(struct sata_rcar_priv *priv) >> +static void sata_rcar_phy_preinit(struct sata_rcar_priv *priv) >> { >> void __iomem *base = priv->base; >> >> @@ -170,6 +196,29 @@ static void sata_rcar_phy_write(struct sata_rcar_priv *priv, u16 reg, u32 val, >> iowrite32(0, base + SATAPHYADDR_REG); >> } >> >> +static void sata_rcar_phy_init(struct sata_rcar_priv *priv) >> +{ >> + sata_rcar_phy_preinit(priv); >> + sata_rcar_phy_write(priv, SATAPCTLR1_REG, 0x00200188, 0); >> + sata_rcar_phy_write(priv, SATAPCTLR1_REG, 0x00200188, 1); >> + sata_rcar_phy_write(priv, SATAPCTLR3_REG, 0x0000A061, 0); >> + sata_rcar_phy_write(priv, SATAPCTLR2_REG, 0x20000000, 0); >> + sata_rcar_phy_write(priv, SATAPCTLR2_REG, 0x20000000, 1); >> + sata_rcar_phy_write(priv, SATAPCTLR4_REG, 0x28E80000, 0); >> +} >> + >> +static void sata_rcar_gen2_phy_init(struct sata_rcar_priv *priv) >> +{ >> + void __iomem *base = priv->base; >> + >> + iowrite32(RCAR_GEN2_PHY_CTL1, base + RCAR_GEN2_PHY_CTL1_REG); >> + iowrite32(RCAR_GEN2_PHY_CTL2, base + RCAR_GEN2_PHY_CTL2_REG); >> + iowrite32(RCAR_GEN2_PHY_CTL3, base + RCAR_GEN2_PHY_CTL3_REG); >> + iowrite32(RCAR_GEN2_PHY_CTL4, base + RCAR_GEN2_PHY_CTL4_REG); >> + iowrite32(RCAR_GEN2_PHY_CTL5 | RCAR_GEN2_PHY_CTL5_DC | >> + RCAR_GEN2_PHY_CTL5_TR, base + RCAR_GEN2_PHY_CTL5_REG); >> +} >> + >> static void sata_rcar_freeze(struct ata_port *ap) >> { >> struct sata_rcar_priv *priv = ap->host->private_data; >> @@ -738,13 +787,17 @@ static void sata_rcar_init_controller(struct ata_host *host) >> u32 val; >> >> /* reset and setup phy */ >> - sata_rcar_phy_initialize(priv); >> - sata_rcar_phy_write(priv, SATAPCTLR1_REG, 0x00200188, 0); >> - sata_rcar_phy_write(priv, SATAPCTLR1_REG, 0x00200188, 1); >> - sata_rcar_phy_write(priv, SATAPCTLR3_REG, 0x0000A061, 0); >> - sata_rcar_phy_write(priv, SATAPCTLR2_REG, 0x20000000, 0); >> - sata_rcar_phy_write(priv, SATAPCTLR2_REG, 0x20000000, 1); >> - sata_rcar_phy_write(priv, SATAPCTLR4_REG, 0x28E80000, 0); >> + switch (priv->type) { >> + case RCAR_GEN2_SATA: >> + sata_rcar_gen2_phy_init(priv); >> + break; >> + case RCAR_SATA: >> + sata_rcar_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); >> @@ -770,8 +823,34 @@ static void sata_rcar_init_controller(struct ata_host *host) >> iowrite32(ATAPI_INT_ENABLE_SATAINT, base + ATAPI_INT_ENABLE_REG); >> } >> >> +static struct of_device_id sata_rcar_match[] = { >> + { >> + .compatible = "renesas,rcar-sata", >> + .data = (void *)RCAR_SATA >> + }, >> + { >> + .compatible = "renesas,sata-r8a7790", >> + .data = (void *)RCAR_GEN2_SATA >> + }, >> + { >> + .compatible = "renesas,sata-r8a7791", >> + .data = (void *)RCAR_GEN2_SATA >> + }, >> + {}, >> +}; >> +MODULE_DEVICE_TABLE(of, sata_rcar_match); >> + >> +static const struct platform_device_id sata_rcar_id_table[] = { >> + { "sata_rcar", RCAR_SATA }, >> + { "sata-r8a7790", RCAR_GEN2_SATA }, >> + { "sata-r8a7791", RCAR_GEN2_SATA }, >> + { }, >> +}; >> +MODULE_DEVICE_TABLE(platform, sata_rcat_id_table); >> + > > I think it would be better to add sata-r8a7779 and > sata-r8a7778 to handle the GEN1 hardware and deprecate rcar-sata. > > Less importantly I think it would be better to name RCAR_SATA > as RCAR_GEN1_SATA. I agree. I think this should be done with a separate patch because it is not related to Gen2 phy. [snip] Thanks, Val. -- To unsubscribe from this list: send the line "unsubscribe linux-sh" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 10/11/2013 01:41 PM, Mark Rutland wrote: > On Fri, Oct 11, 2013 at 02:00:35AM +0100, Simon Horman wrote: >> [ CCed devicetree@vger.kernel.org as this involves DT compatibility strings ] > > Cheers! > Hi Mark, >> >> On Thu, Oct 10, 2013 at 11:08:03PM +0400, Valentine Barshak wrote: >>> RCAR Gen2 SoC has a different phy which is not compatible with >>> the older H1/M1 versions. This adds OF/platform device table >>> and PHY initialization callbacks for H2/M2 (Gen2) SoC. > > Is the PHY combined with the rest of the controller, or are they > logically separate components in the SoC? I note that the Calxeda > Highbank SATA controller driver treats the phy and the controller as > separate entities, and describes the way the two are attached (though > the driver handles both). Would a similar approach work here? > It seems to be described in the docs as a single entity with the rest of the controller. The phy registers are in the same address space block. We don't need to describe how the phy is attached to the SATA controller. In the future we may need some phy-specific configuration in the device tree. For example, to describe how clock generator is connected to the controller. I think this could be done with optional properties added to the SATA node if needed. > [...] > >>> +static struct of_device_id sata_rcar_match[] = { >>> + { >>> + .compatible = "renesas,rcar-sata", >>> + .data = (void *)RCAR_SATA >>> + }, >>> + { >>> + .compatible = "renesas,sata-r8a7790", >>> + .data = (void *)RCAR_GEN2_SATA >>> + }, >>> + { >>> + .compatible = "renesas,sata-r8a7791", >>> + .data = (void *)RCAR_GEN2_SATA >>> + }, >>> + {}, > > These bindings will need documentation. A grep of any of these in > mainline's Documentation/devicetree shows up nothing (not even the > existing "renesas,rcar-sata" string used by the driver. Indeed. Since we need to adjust rcar-sata bindings and add documentation for it, which is not really related to Gen2 phy support, looks like it will be a separate patch. > > Thanks, > Mark. > Thanks, Val. -- To unsubscribe from this list: send the line "unsubscribe linux-sh" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Fri, Oct 11, 2013 at 11:54:52AM +0100, Valentine wrote: > On 10/11/2013 01:41 PM, Mark Rutland wrote: > > On Fri, Oct 11, 2013 at 02:00:35AM +0100, Simon Horman wrote: > >> [ CCed devicetree@vger.kernel.org as this involves DT compatibility strings ] > > > > Cheers! > > > > Hi Mark, > > >> > >> On Thu, Oct 10, 2013 at 11:08:03PM +0400, Valentine Barshak wrote: > >>> RCAR Gen2 SoC has a different phy which is not compatible with > >>> the older H1/M1 versions. This adds OF/platform device table > >>> and PHY initialization callbacks for H2/M2 (Gen2) SoC. > > > > Is the PHY combined with the rest of the controller, or are they > > logically separate components in the SoC? I note that the Calxeda > > Highbank SATA controller driver treats the phy and the controller as > > separate entities, and describes the way the two are attached (though > > the driver handles both). Would a similar approach work here? > > > > It seems to be described in the docs as a single entity with the rest of > the controller. The phy registers are in the same address space block. > We don't need to describe how the phy is attached to the SATA > controller. In the future we may need some phy-specific configuration in > the device tree. For example, to describe how clock generator is > connected to the controller. I think this could be done with optional > properties added to the SATA node if needed. As they're tightly coupled and share the same register block, I guess describing them together is ok. Is that clock example hypothetical, or something we _will_ need in future? > > > [...] > > > >>> +static struct of_device_id sata_rcar_match[] = { > >>> + { > >>> + .compatible = "renesas,rcar-sata", > >>> + .data = (void *)RCAR_SATA > >>> + }, > >>> + { > >>> + .compatible = "renesas,sata-r8a7790", > >>> + .data = (void *)RCAR_GEN2_SATA > >>> + }, > >>> + { > >>> + .compatible = "renesas,sata-r8a7791", > >>> + .data = (void *)RCAR_GEN2_SATA > >>> + }, > >>> + {}, > > > > These bindings will need documentation. A grep of any of these in > > mainline's Documentation/devicetree shows up nothing (not even the > > existing "renesas,rcar-sata" string used by the driver. > > Indeed. > Since we need to adjust rcar-sata bindings and add documentation for it, > which is not really related to Gen2 phy support, looks like it will be a > separate patch. If you're adding strings, there should be documentation. While it may be a separate patch, we should _not_ add compatible strings to the kernel without documentation. I'd like to see the documentation patch first. Thanks, Mark. -- To unsubscribe from this list: send the line "unsubscribe linux-sh" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 10/11/2013 06:47 PM, Mark Rutland wrote: > On Fri, Oct 11, 2013 at 11:54:52AM +0100, Valentine wrote: >> On 10/11/2013 01:41 PM, Mark Rutland wrote: >>> On Fri, Oct 11, 2013 at 02:00:35AM +0100, Simon Horman wrote: >>>> [ CCed devicetree@vger.kernel.org as this involves DT compatibility strings ] >>> >>> Cheers! >>> >> >> Hi Mark, >> >>>> >>>> On Thu, Oct 10, 2013 at 11:08:03PM +0400, Valentine Barshak wrote: >>>>> RCAR Gen2 SoC has a different phy which is not compatible with >>>>> the older H1/M1 versions. This adds OF/platform device table >>>>> and PHY initialization callbacks for H2/M2 (Gen2) SoC. >>> >>> Is the PHY combined with the rest of the controller, or are they >>> logically separate components in the SoC? I note that the Calxeda >>> Highbank SATA controller driver treats the phy and the controller as >>> separate entities, and describes the way the two are attached (though >>> the driver handles both). Would a similar approach work here? >>> >> >> It seems to be described in the docs as a single entity with the rest of >> the controller. The phy registers are in the same address space block. >> We don't need to describe how the phy is attached to the SATA >> controller. In the future we may need some phy-specific configuration in >> the device tree. For example, to describe how clock generator is >> connected to the controller. I think this could be done with optional >> properties added to the SATA node if needed. > > As they're tightly coupled and share the same register block, I guess > describing them together is ok. Is that clock example hypothetical, or > something we _will_ need in future? According to the manual there's some configuration possible, though it's not verbosely described: * SSCG (on/off) * Clock connection (AC/DC) * Termination Resistor (on/off) AFAIU, this describes how the external clock generator is connected. All available Gen2 boards use the same generator type and the same "default" phy settings (SSCG off/DC/TR on). It is possible set spread spectrum with a switch on the board (it is disabled by default) I don't know if we will need to change those settings in the future. Probably this could be done as a separate patch if needed. > >> >>> [...] >>> >>>>> +static struct of_device_id sata_rcar_match[] = { >>>>> + { >>>>> + .compatible = "renesas,rcar-sata", >>>>> + .data = (void *)RCAR_SATA >>>>> + }, >>>>> + { >>>>> + .compatible = "renesas,sata-r8a7790", >>>>> + .data = (void *)RCAR_GEN2_SATA >>>>> + }, >>>>> + { >>>>> + .compatible = "renesas,sata-r8a7791", >>>>> + .data = (void *)RCAR_GEN2_SATA >>>>> + }, >>>>> + {}, >>> >>> These bindings will need documentation. A grep of any of these in >>> mainline's Documentation/devicetree shows up nothing (not even the >>> existing "renesas,rcar-sata" string used by the driver. >> >> Indeed. >> Since we need to adjust rcar-sata bindings and add documentation for it, >> which is not really related to Gen2 phy support, looks like it will be a >> separate patch. > > If you're adding strings, there should be documentation. While it may be > a separate patch, we should _not_ add compatible strings to the kernel > without documentation. I'd like to see the documentation patch first. Right. I meant that there should be a separate patch that adds documentation for the already existing binding. I think I'll make a couple of patches then: * Adjust and document the existing bindings as Simon has suggested * Add Gen2 support and document Ge2 bindings. > > Thanks, > Mark. > Thanks, Val. -- To unsubscribe from this list: send the line "unsubscribe linux-sh" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Hello. On 11-10-2013 3:00, Simon Horman wrote: > [ CCed devicetree@vger.kernel.org as this involves DT compatibility strings ] Note that I did CC then-existing devicetree-discuss list when posting the original driver but we got no feedback from there. That's why the binding documentation is missing. >> RCAR Gen2 SoC has a different phy which is not compatible with Hm, the SoCs family is called R-Car, is it so hard to remember? :-) >> the older H1/M1 versions. This adds OF/platform device table >> and PHY initialization callbacks for H2/M2 (Gen2) SoC. [...] >> PHY initialization method is chosen based on the device id. >> Default PHY settings are applied for Gen2 SoC, which should >> suit the available Gen2 boards. >> Signed-off-by: Valentine Barshak <valentine.barshak@cogentembedded.com> [skip overquoating, grr :-] >> @@ -770,8 +823,34 @@ static void sata_rcar_init_controller(struct ata_host *host) >> iowrite32(ATAPI_INT_ENABLE_SATAINT, base + ATAPI_INT_ENABLE_REG); >> } >> >> +static struct of_device_id sata_rcar_match[] = { >> + { >> + .compatible = "renesas,rcar-sata", >> + .data = (void *)RCAR_SATA >> + }, >> + { >> + .compatible = "renesas,sata-r8a7790", >> + .data = (void *)RCAR_GEN2_SATA >> + }, >> + { >> + .compatible = "renesas,sata-r8a7791", >> + .data = (void *)RCAR_GEN2_SATA >> + }, >> + {}, >> +}; >> +MODULE_DEVICE_TABLE(of, sata_rcar_match); >> + >> +static const struct platform_device_id sata_rcar_id_table[] = { >> + { "sata_rcar", RCAR_SATA }, >> + { "sata-r8a7790", RCAR_GEN2_SATA }, >> + { "sata-r8a7791", RCAR_GEN2_SATA }, >> + { }, >> +}; >> +MODULE_DEVICE_TABLE(platform, sata_rcat_id_table); >> + > I think it would be better to add sata-r8a7779 and > sata-r8a7778 to handle the GEN1 hardware and deprecate rcar-sata. R8A7778 doesn't have the SATA controller AFAIR. > Less importantly I think it would be better to name RCAR_SATA > as RCAR_GEN1_SATA. Now that we have Gen2, yes. WBR, Sergei -- To unsubscribe from this list: send the line "unsubscribe linux-sh" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/drivers/ata/sata_rcar.c b/drivers/ata/sata_rcar.c index c2d95e9..45af29f 100644 --- a/drivers/ata/sata_rcar.c +++ b/drivers/ata/sata_rcar.c @@ -15,6 +15,7 @@ #include <linux/module.h> #include <linux/ata.h> #include <linux/libata.h> +#include <linux/of_device.h> #include <linux/platform_device.h> #include <linux/clk.h> #include <linux/err.h> @@ -118,17 +119,42 @@ #define SATAPCTLR3_REG 0x5A #define SATAPCTLR4_REG 0x60 +/* Gen2 Physical Layer Control Registers */ +#define RCAR_GEN2_PHY_CTL1_REG 0x1704 +#define RCAR_GEN2_PHY_CTL1 0x34180002 +#define RCAR_GEN2_PHY_CTL1_SS 0xC180 /* Spread Spectrum */ + +#define RCAR_GEN2_PHY_CTL2_REG 0x170C +#define RCAR_GEN2_PHY_CTL2 0x00002303 + +#define RCAR_GEN2_PHY_CTL3_REG 0x171C +#define RCAR_GEN2_PHY_CTL3 0x000B0194 + +#define RCAR_GEN2_PHY_CTL4_REG 0x1724 +#define RCAR_GEN2_PHY_CTL4 0x00030994 + +#define RCAR_GEN2_PHY_CTL5_REG 0x1740 +#define RCAR_GEN2_PHY_CTL5 0x03004001 +#define RCAR_GEN2_PHY_CTL5_DC BIT(1) /* DC connection */ +#define RCAR_GEN2_PHY_CTL5_TR BIT(2) /* Termination Resistor */ + /* Descriptor table word 0 bit (when DTA32M = 1) */ #define SATA_RCAR_DTEND BIT(0) #define SATA_RCAR_DMA_BOUNDARY 0x1FFFFFFEUL +enum sata_rcar_type { + RCAR_SATA, + RCAR_GEN2_SATA, +}; + struct sata_rcar_priv { void __iomem *base; struct clk *clk; + enum sata_rcar_type type; }; -static void sata_rcar_phy_initialize(struct sata_rcar_priv *priv) +static void sata_rcar_phy_preinit(struct sata_rcar_priv *priv) { void __iomem *base = priv->base; @@ -170,6 +196,29 @@ static void sata_rcar_phy_write(struct sata_rcar_priv *priv, u16 reg, u32 val, iowrite32(0, base + SATAPHYADDR_REG); } +static void sata_rcar_phy_init(struct sata_rcar_priv *priv) +{ + sata_rcar_phy_preinit(priv); + sata_rcar_phy_write(priv, SATAPCTLR1_REG, 0x00200188, 0); + sata_rcar_phy_write(priv, SATAPCTLR1_REG, 0x00200188, 1); + sata_rcar_phy_write(priv, SATAPCTLR3_REG, 0x0000A061, 0); + sata_rcar_phy_write(priv, SATAPCTLR2_REG, 0x20000000, 0); + sata_rcar_phy_write(priv, SATAPCTLR2_REG, 0x20000000, 1); + sata_rcar_phy_write(priv, SATAPCTLR4_REG, 0x28E80000, 0); +} + +static void sata_rcar_gen2_phy_init(struct sata_rcar_priv *priv) +{ + void __iomem *base = priv->base; + + iowrite32(RCAR_GEN2_PHY_CTL1, base + RCAR_GEN2_PHY_CTL1_REG); + iowrite32(RCAR_GEN2_PHY_CTL2, base + RCAR_GEN2_PHY_CTL2_REG); + iowrite32(RCAR_GEN2_PHY_CTL3, base + RCAR_GEN2_PHY_CTL3_REG); + iowrite32(RCAR_GEN2_PHY_CTL4, base + RCAR_GEN2_PHY_CTL4_REG); + iowrite32(RCAR_GEN2_PHY_CTL5 | RCAR_GEN2_PHY_CTL5_DC | + RCAR_GEN2_PHY_CTL5_TR, base + RCAR_GEN2_PHY_CTL5_REG); +} + static void sata_rcar_freeze(struct ata_port *ap) { struct sata_rcar_priv *priv = ap->host->private_data; @@ -738,13 +787,17 @@ static void sata_rcar_init_controller(struct ata_host *host) u32 val; /* reset and setup phy */ - sata_rcar_phy_initialize(priv); - sata_rcar_phy_write(priv, SATAPCTLR1_REG, 0x00200188, 0); - sata_rcar_phy_write(priv, SATAPCTLR1_REG, 0x00200188, 1); - sata_rcar_phy_write(priv, SATAPCTLR3_REG, 0x0000A061, 0); - sata_rcar_phy_write(priv, SATAPCTLR2_REG, 0x20000000, 0); - sata_rcar_phy_write(priv, SATAPCTLR2_REG, 0x20000000, 1); - sata_rcar_phy_write(priv, SATAPCTLR4_REG, 0x28E80000, 0); + switch (priv->type) { + case RCAR_GEN2_SATA: + sata_rcar_gen2_phy_init(priv); + break; + case RCAR_SATA: + sata_rcar_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); @@ -770,8 +823,34 @@ static void sata_rcar_init_controller(struct ata_host *host) iowrite32(ATAPI_INT_ENABLE_SATAINT, base + ATAPI_INT_ENABLE_REG); } +static struct of_device_id sata_rcar_match[] = { + { + .compatible = "renesas,rcar-sata", + .data = (void *)RCAR_SATA + }, + { + .compatible = "renesas,sata-r8a7790", + .data = (void *)RCAR_GEN2_SATA + }, + { + .compatible = "renesas,sata-r8a7791", + .data = (void *)RCAR_GEN2_SATA + }, + {}, +}; +MODULE_DEVICE_TABLE(of, sata_rcar_match); + +static const struct platform_device_id sata_rcar_id_table[] = { + { "sata_rcar", RCAR_SATA }, + { "sata-r8a7790", RCAR_GEN2_SATA }, + { "sata-r8a7791", RCAR_GEN2_SATA }, + { }, +}; +MODULE_DEVICE_TABLE(platform, sata_rcat_id_table); + static int sata_rcar_probe(struct platform_device *pdev) { + const struct of_device_id *of_id; struct ata_host *host; struct sata_rcar_priv *priv; struct resource *mem; @@ -787,6 +866,12 @@ static int sata_rcar_probe(struct platform_device *pdev) if (!priv) return -ENOMEM; + of_id = of_match_device(sata_rcar_match, &pdev->dev); + if (of_id) + priv->type = (enum sata_rcar_type)of_id->data; + else + priv->type = platform_get_device_id(pdev)->driver_data; + priv->clk = devm_clk_get(&pdev->dev, NULL); if (IS_ERR(priv->clk)) { dev_err(&pdev->dev, "failed to get access to sata clock\n"); @@ -892,15 +977,10 @@ static const struct dev_pm_ops sata_rcar_pm_ops = { }; #endif -static struct of_device_id sata_rcar_match[] = { - { .compatible = "renesas,rcar-sata", }, - {}, -}; -MODULE_DEVICE_TABLE(of, sata_rcar_match); - static struct platform_driver sata_rcar_driver = { .probe = sata_rcar_probe, .remove = sata_rcar_remove, + .id_table = sata_rcar_id_table, .driver = { .name = DRV_NAME, .owner = THIS_MODULE,
RCAR Gen2 SoC has a different phy which is not compatible with the older H1/M1 versions. This adds OF/platform device table and PHY initialization callbacks for H2/M2 (Gen2) SoC. PHY initialization method is chosen based on the device id. Default PHY settings are applied for Gen2 SoC, which should suit the available Gen2 boards. Signed-off-by: Valentine Barshak <valentine.barshak@cogentembedded.com> --- drivers/ata/sata_rcar.c | 108 +++++++++++++++++++++++++++++++++++++++++------- 1 file changed, 94 insertions(+), 14 deletions(-)