| Message ID | 1406193450-17283-5-git-send-email-antoine.tenart@free-electrons.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
Hi, Thanks, this version is almost perfect, unfortunately a second review has found one small issue in it, see inline comment below. On 07/24/2014 11:17 AM, Antoine Ténart wrote: > The current implementation of the libahci does not allow to use multiple > PHYs. This patch adds the support of multiple PHYs by the libahci while > keeping the old bindings valid for device tree compatibility. > > This introduce a new way of defining SATA ports in the device tree, with > one port per sub-node. This as the advantage of allowing a per port > configuration. Because some ports may be accessible but disabled in the > device tree, the port_map mask is computed automatically when using > this. > > Signed-off-by: Antoine Ténart <antoine.tenart@free-electrons.com> > --- > drivers/ata/ahci.h | 7 +- > drivers/ata/libahci_platform.c | 190 ++++++++++++++++++++++++++++++++--------- > 2 files changed, 157 insertions(+), 40 deletions(-) > > diff --git a/drivers/ata/ahci.h b/drivers/ata/ahci.h > index cb8d58926851..47de53284ad7 100644 > --- a/drivers/ata/ahci.h > +++ b/drivers/ata/ahci.h > @@ -332,7 +332,12 @@ struct ahci_host_priv { > bool got_runtime_pm; /* Did we do pm_runtime_get? */ > struct clk *clks[AHCI_MAX_CLKS]; /* Optional */ > struct regulator *target_pwr; /* Optional */ > - struct phy *phy; /* If platform uses phy */ > + /* > + * If platform uses PHYs. There is a 1:1 relation between the port number and > + * the PHY position in this array. > + */ > + struct phy **phys; > + unsigned nports; /* Number of ports */ > void *plat_data; /* Other platform data */ > /* > * Optional ahci_start_engine override, if not set this gets set to the > diff --git a/drivers/ata/libahci_platform.c b/drivers/ata/libahci_platform.c > index db9b90d876dd..095df56432d9 100644 > --- a/drivers/ata/libahci_platform.c > +++ b/drivers/ata/libahci_platform.c > @@ -39,6 +39,67 @@ static struct scsi_host_template ahci_platform_sht = { > }; > > /** > + * ahci_platform_enable_phys - Enable PHYs > + * @hpriv: host private area to store config values > + * > + * This function enables all the PHYs found in hpriv->phys, if any. > + * If a PHY fails to be enabled, it disables all the PHYs already > + * enabled in reverse order and returns an error. > + * > + * RETURNS: > + * 0 on success otherwise a negative error code > + */ > +int ahci_platform_enable_phys(struct ahci_host_priv *hpriv) > +{ > + int rc, i; > + > + for (i = 0; i < hpriv->nports; i++) { > + if (!hpriv->phys[i]) > + continue; > + > + rc = phy_init(hpriv->phys[i]); > + if (rc) > + goto disable_phys; > + > + rc = phy_power_on(hpriv->phys[i]); > + if (rc) { > + phy_exit(hpriv->phys[i]); > + goto disable_phys; > + } > + } > + > + return 0; > + > +disable_phys: > + while (--i >= 0) { > + phy_power_off(hpriv->phys[i]); > + phy_exit(hpriv->phys[i]); > + } > + return rc; > +} > +EXPORT_SYMBOL_GPL(ahci_platform_enable_phys); > + > +/** > + * ahci_platform_disable_phys - Enable PHYs > + * @hpriv: host private area to store config values > + * > + * This function disables all PHYs found in hpriv->phys. > + */ > +void ahci_platform_disable_phys(struct ahci_host_priv *hpriv) > +{ > + int i; > + > + for (i = 0; i < hpriv->nports; i++) { > + if (!hpriv->phys[i]) > + continue; > + > + phy_power_off(hpriv->phys[i]); > + phy_exit(hpriv->phys[i]); > + } > +} > +EXPORT_SYMBOL_GPL(ahci_platform_disable_phys); > + > +/** > * ahci_platform_enable_clks - Enable platform clocks > * @hpriv: host private area to store config values > * > @@ -92,7 +153,7 @@ EXPORT_SYMBOL_GPL(ahci_platform_disable_clks); > * following order: > * 1) Regulator > * 2) Clocks (through ahci_platform_enable_clks) > - * 3) Phy > + * 3) Phys > * > * If resource enabling fails at any point the previous enabled resources > * are disabled in reverse order. > @@ -114,17 +175,9 @@ int ahci_platform_enable_resources(struct ahci_host_priv *hpriv) > if (rc) > goto disable_regulator; > > - if (hpriv->phy) { > - rc = phy_init(hpriv->phy); > - if (rc) > - goto disable_clks; > - > - rc = phy_power_on(hpriv->phy); > - if (rc) { > - phy_exit(hpriv->phy); > - goto disable_clks; > - } > - } > + rc = ahci_platform_enable_phys(hpriv); > + if (rc) > + goto disable_clks; > > return 0; > > @@ -144,16 +197,13 @@ EXPORT_SYMBOL_GPL(ahci_platform_enable_resources); > * > * This function disables all ahci_platform managed resources in the > * following order: > - * 1) Phy > + * 1) Phys > * 2) Clocks (through ahci_platform_disable_clks) > * 3) Regulator > */ > void ahci_platform_disable_resources(struct ahci_host_priv *hpriv) > { > - if (hpriv->phy) { > - phy_power_off(hpriv->phy); > - phy_exit(hpriv->phy); > - } > + ahci_platform_disable_phys(hpriv); > > ahci_platform_disable_clks(hpriv); > > @@ -187,7 +237,7 @@ static void ahci_platform_put_resources(struct device *dev, void *res) > * 2) regulator for controlling the targets power (optional) > * 3) 0 - AHCI_MAX_CLKS clocks, as specified in the devs devicetree node, > * or for non devicetree enabled platforms a single clock > - * 4) phy (optional) > + * 4) phys (optional) > * > * RETURNS: > * The allocated ahci_host_priv on success, otherwise an ERR_PTR value > @@ -197,7 +247,9 @@ struct ahci_host_priv *ahci_platform_get_resources(struct platform_device *pdev) > struct device *dev = &pdev->dev; > struct ahci_host_priv *hpriv; > struct clk *clk; > - int i, rc = -ENOMEM; > + struct device_node *child; > + int i, enabled_ports = 0, rc = -ENOMEM; > + u32 mask_port_map = 0; > > if (!devres_open_group(dev, NULL, GFP_KERNEL)) > return ERR_PTR(-ENOMEM); > @@ -246,27 +298,87 @@ struct ahci_host_priv *ahci_platform_get_resources(struct platform_device *pdev) > hpriv->clks[i] = clk; > } > > - hpriv->phy = devm_phy_get(dev, "sata-phy"); > - if (IS_ERR(hpriv->phy)) { > - rc = PTR_ERR(hpriv->phy); > - switch (rc) { > - case -ENOSYS: > - /* No PHY support. Check if PHY is required. */ > - if (of_find_property(dev->of_node, "phys", NULL)) { > - dev_err(dev, "couldn't get sata-phy: ENOSYS\n"); > + hpriv->nports = of_get_child_count(dev->of_node); > + > + if (hpriv->nports) { > + hpriv->phys = devm_kzalloc(dev, > + hpriv->nports * sizeof(*hpriv->phys), > + GFP_KERNEL); > + if (!hpriv->phys) { > + rc = -ENOMEM; > + goto err_out; > + } > + > + for_each_child_of_node(dev->of_node, child) { > + u32 port; > + > + if (!of_device_is_available(child)) > + continue; > + > + if (of_property_read_u32(child, "reg", &port)) { > + rc = -EINVAL; > goto err_out; > } > - case -ENODEV: > - /* continue normally */ > - hpriv->phy = NULL; > - break; > > - case -EPROBE_DEFER: > - goto err_out; > + if (port >= hpriv->nports) { > + dev_warn(dev, "invalid port number %d\n", port); > + continue; > + } > > - default: > - dev_err(dev, "couldn't get sata-phy\n"); > - goto err_out; > + mask_port_map |= BIT(port); > + > + hpriv->phys[port] = devm_of_phy_get(dev, child, NULL); > + if (IS_ERR(hpriv->phys[port])) { > + rc = PTR_ERR(hpriv->phys[port]); > + dev_err(dev, > + "couldn't get PHY in node %s: %d\n", > + child->name, rc); > + goto err_out; > + } > + > + enabled_ports++; > + } > + if (!enabled_ports) { > + dev_warn(dev, "No port enabled\n"); > + return ERR_PTR(-ENODEV); This should be: rc = -ENODEV; goto err_out; Sorry for not catching that sooner. Other then that the entire series looks good and is: Acked-by: Hans de Goede <hdegoede@redhat.com> Regards, Hans > + } > + > + if (!hpriv->mask_port_map) > + hpriv->mask_port_map = mask_port_map; > + } else { > + /* > + * If no sub-node was found, keep this for device tree > + * compatibility > + */ > + struct phy *phy = devm_phy_get(dev, "sata-phy"); > + if (!IS_ERR(phy)) { > + hpriv->phys = devm_kzalloc(dev, sizeof(*hpriv->phys), > + GFP_KERNEL); > + if (!hpriv->phys) { > + rc = -ENOMEM; > + goto err_out; > + } > + > + hpriv->phys[0] = phy; > + hpriv->nports = 1; > + } else { > + rc = PTR_ERR(phy); > + switch (rc) { > + case -ENOSYS: > + /* No PHY support. Check if PHY is required. */ > + if (of_find_property(dev->of_node, "phys", NULL)) { > + dev_err(dev, "couldn't get sata-phy: ENOSYS\n"); > + goto err_out; > + } > + case -ENODEV: > + /* continue normally */ > + hpriv->phys = NULL; > + break; > + > + default: > + goto err_out; > + > + } > } > } > > @@ -291,7 +403,7 @@ EXPORT_SYMBOL_GPL(ahci_platform_get_resources); > * @host_flags: ahci host flags used in ahci_host_priv > * > * This function does all the usual steps needed to bring up an > - * ahci-platform host, note any necessary resources (ie clks, phy, etc.) > + * ahci-platform host, note any necessary resources (ie clks, phys, etc.) > * must be initialized / enabled before calling this. > * > * RETURNS: > @@ -395,7 +507,7 @@ static void ahci_host_stop(struct ata_host *host) > * @dev: device pointer for the host > * > * This function does all the usual steps needed to suspend an > - * ahci-platform host, note any necessary resources (ie clks, phy, etc.) > + * ahci-platform host, note any necessary resources (ie clks, phys, etc.) > * must be disabled after calling this. > * > * RETURNS: > @@ -432,7 +544,7 @@ EXPORT_SYMBOL_GPL(ahci_platform_suspend_host); > * @dev: device pointer for the host > * > * This function does all the usual steps needed to resume an ahci-platform > - * host, note any necessary resources (ie clks, phy, etc.) must be > + * host, note any necessary resources (ie clks, phys, etc.) must be > * initialized / enabled before calling this. > * > * RETURNS: >
On Mon, Jul 28, 2014 at 12:29:56PM +0200, Hans de Goede wrote: ... > > + if (!enabled_ports) { > > + dev_warn(dev, "No port enabled\n"); > > + return ERR_PTR(-ENODEV); > > This should be: > rc = -ENODEV; > goto err_out; > > Sorry for not catching that sooner. > > Other then that the entire series looks good and is: > > Acked-by: Hans de Goede <hdegoede@redhat.com> Antoine, can you please post an updated version of this patch? No need to repost the whole series. Also, how should the series be routed? Thanks.
Hi, On Mon, Jul 28, 2014 at 01:27:24PM -0400, Tejun Heo wrote: > On Mon, Jul 28, 2014 at 12:29:56PM +0200, Hans de Goede wrote: > ... > > > + if (!enabled_ports) { > > > + dev_warn(dev, "No port enabled\n"); > > > + return ERR_PTR(-ENODEV); > > > > This should be: > > rc = -ENODEV; > > goto err_out; > > > > Sorry for not catching that sooner. > > > > Other then that the entire series looks good and is: > > > > Acked-by: Hans de Goede <hdegoede@redhat.com> > > Antoine, can you please post an updated version of this patch? No > need to repost the whole series. Also, how should the series be > routed? Sure, I'm doing this right away. Regarding the series, Kishon already took patches 1-2. Antoine
On Thursday 24 July 2014 02:47 PM, Antoine Ténart wrote: > The current implementation of the libahci does not allow to use multiple > PHYs. This patch adds the support of multiple PHYs by the libahci while > keeping the old bindings valid for device tree compatibility. > > This introduce a new way of defining SATA ports in the device tree, with > one port per sub-node. This as the advantage of allowing a per port > configuration. Because some ports may be accessible but disabled in the > device tree, the port_map mask is computed automatically when using > this. > > Signed-off-by: Antoine Ténart <antoine.tenart@free-electrons.com> FWIW: Acked-by: Kishon Vijay Abraham I <kishon@ti.com> > --- > drivers/ata/ahci.h | 7 +- > drivers/ata/libahci_platform.c | 190 ++++++++++++++++++++++++++++++++--------- > 2 files changed, 157 insertions(+), 40 deletions(-) > > diff --git a/drivers/ata/ahci.h b/drivers/ata/ahci.h > index cb8d58926851..47de53284ad7 100644 > --- a/drivers/ata/ahci.h > +++ b/drivers/ata/ahci.h > @@ -332,7 +332,12 @@ struct ahci_host_priv { > bool got_runtime_pm; /* Did we do pm_runtime_get? */ > struct clk *clks[AHCI_MAX_CLKS]; /* Optional */ > struct regulator *target_pwr; /* Optional */ > - struct phy *phy; /* If platform uses phy */ > + /* > + * If platform uses PHYs. There is a 1:1 relation between the port number and > + * the PHY position in this array. > + */ > + struct phy **phys; > + unsigned nports; /* Number of ports */ > void *plat_data; /* Other platform data */ > /* > * Optional ahci_start_engine override, if not set this gets set to the > diff --git a/drivers/ata/libahci_platform.c b/drivers/ata/libahci_platform.c > index db9b90d876dd..095df56432d9 100644 > --- a/drivers/ata/libahci_platform.c > +++ b/drivers/ata/libahci_platform.c > @@ -39,6 +39,67 @@ static struct scsi_host_template ahci_platform_sht = { > }; > > /** > + * ahci_platform_enable_phys - Enable PHYs > + * @hpriv: host private area to store config values > + * > + * This function enables all the PHYs found in hpriv->phys, if any. > + * If a PHY fails to be enabled, it disables all the PHYs already > + * enabled in reverse order and returns an error. > + * > + * RETURNS: > + * 0 on success otherwise a negative error code > + */ > +int ahci_platform_enable_phys(struct ahci_host_priv *hpriv) > +{ > + int rc, i; > + > + for (i = 0; i < hpriv->nports; i++) { > + if (!hpriv->phys[i]) > + continue; > + > + rc = phy_init(hpriv->phys[i]); > + if (rc) > + goto disable_phys; > + > + rc = phy_power_on(hpriv->phys[i]); > + if (rc) { > + phy_exit(hpriv->phys[i]); > + goto disable_phys; > + } > + } > + > + return 0; > + > +disable_phys: > + while (--i >= 0) { > + phy_power_off(hpriv->phys[i]); > + phy_exit(hpriv->phys[i]); > + } > + return rc; > +} > +EXPORT_SYMBOL_GPL(ahci_platform_enable_phys); > + > +/** > + * ahci_platform_disable_phys - Enable PHYs > + * @hpriv: host private area to store config values > + * > + * This function disables all PHYs found in hpriv->phys. > + */ > +void ahci_platform_disable_phys(struct ahci_host_priv *hpriv) > +{ > + int i; > + > + for (i = 0; i < hpriv->nports; i++) { > + if (!hpriv->phys[i]) > + continue; > + > + phy_power_off(hpriv->phys[i]); > + phy_exit(hpriv->phys[i]); > + } > +} > +EXPORT_SYMBOL_GPL(ahci_platform_disable_phys); > + > +/** > * ahci_platform_enable_clks - Enable platform clocks > * @hpriv: host private area to store config values > * > @@ -92,7 +153,7 @@ EXPORT_SYMBOL_GPL(ahci_platform_disable_clks); > * following order: > * 1) Regulator > * 2) Clocks (through ahci_platform_enable_clks) > - * 3) Phy > + * 3) Phys > * > * If resource enabling fails at any point the previous enabled resources > * are disabled in reverse order. > @@ -114,17 +175,9 @@ int ahci_platform_enable_resources(struct ahci_host_priv *hpriv) > if (rc) > goto disable_regulator; > > - if (hpriv->phy) { > - rc = phy_init(hpriv->phy); > - if (rc) > - goto disable_clks; > - > - rc = phy_power_on(hpriv->phy); > - if (rc) { > - phy_exit(hpriv->phy); > - goto disable_clks; > - } > - } > + rc = ahci_platform_enable_phys(hpriv); > + if (rc) > + goto disable_clks; > > return 0; > > @@ -144,16 +197,13 @@ EXPORT_SYMBOL_GPL(ahci_platform_enable_resources); > * > * This function disables all ahci_platform managed resources in the > * following order: > - * 1) Phy > + * 1) Phys > * 2) Clocks (through ahci_platform_disable_clks) > * 3) Regulator > */ > void ahci_platform_disable_resources(struct ahci_host_priv *hpriv) > { > - if (hpriv->phy) { > - phy_power_off(hpriv->phy); > - phy_exit(hpriv->phy); > - } > + ahci_platform_disable_phys(hpriv); > > ahci_platform_disable_clks(hpriv); > > @@ -187,7 +237,7 @@ static void ahci_platform_put_resources(struct device *dev, void *res) > * 2) regulator for controlling the targets power (optional) > * 3) 0 - AHCI_MAX_CLKS clocks, as specified in the devs devicetree node, > * or for non devicetree enabled platforms a single clock > - * 4) phy (optional) > + * 4) phys (optional) > * > * RETURNS: > * The allocated ahci_host_priv on success, otherwise an ERR_PTR value > @@ -197,7 +247,9 @@ struct ahci_host_priv *ahci_platform_get_resources(struct platform_device *pdev) > struct device *dev = &pdev->dev; > struct ahci_host_priv *hpriv; > struct clk *clk; > - int i, rc = -ENOMEM; > + struct device_node *child; > + int i, enabled_ports = 0, rc = -ENOMEM; > + u32 mask_port_map = 0; > > if (!devres_open_group(dev, NULL, GFP_KERNEL)) > return ERR_PTR(-ENOMEM); > @@ -246,27 +298,87 @@ struct ahci_host_priv *ahci_platform_get_resources(struct platform_device *pdev) > hpriv->clks[i] = clk; > } > > - hpriv->phy = devm_phy_get(dev, "sata-phy"); > - if (IS_ERR(hpriv->phy)) { > - rc = PTR_ERR(hpriv->phy); > - switch (rc) { > - case -ENOSYS: > - /* No PHY support. Check if PHY is required. */ > - if (of_find_property(dev->of_node, "phys", NULL)) { > - dev_err(dev, "couldn't get sata-phy: ENOSYS\n"); > + hpriv->nports = of_get_child_count(dev->of_node); > + > + if (hpriv->nports) { > + hpriv->phys = devm_kzalloc(dev, > + hpriv->nports * sizeof(*hpriv->phys), > + GFP_KERNEL); > + if (!hpriv->phys) { > + rc = -ENOMEM; > + goto err_out; > + } > + > + for_each_child_of_node(dev->of_node, child) { > + u32 port; > + > + if (!of_device_is_available(child)) > + continue; > + > + if (of_property_read_u32(child, "reg", &port)) { > + rc = -EINVAL; > goto err_out; > } > - case -ENODEV: > - /* continue normally */ > - hpriv->phy = NULL; > - break; > > - case -EPROBE_DEFER: > - goto err_out; > + if (port >= hpriv->nports) { > + dev_warn(dev, "invalid port number %d\n", port); > + continue; > + } > > - default: > - dev_err(dev, "couldn't get sata-phy\n"); > - goto err_out; > + mask_port_map |= BIT(port); > + > + hpriv->phys[port] = devm_of_phy_get(dev, child, NULL); > + if (IS_ERR(hpriv->phys[port])) { > + rc = PTR_ERR(hpriv->phys[port]); > + dev_err(dev, > + "couldn't get PHY in node %s: %d\n", > + child->name, rc); > + goto err_out; > + } > + > + enabled_ports++; > + } > + if (!enabled_ports) { > + dev_warn(dev, "No port enabled\n"); > + return ERR_PTR(-ENODEV); > + } > + > + if (!hpriv->mask_port_map) > + hpriv->mask_port_map = mask_port_map; > + } else { > + /* > + * If no sub-node was found, keep this for device tree > + * compatibility > + */ > + struct phy *phy = devm_phy_get(dev, "sata-phy"); > + if (!IS_ERR(phy)) { > + hpriv->phys = devm_kzalloc(dev, sizeof(*hpriv->phys), > + GFP_KERNEL); > + if (!hpriv->phys) { > + rc = -ENOMEM; > + goto err_out; > + } > + > + hpriv->phys[0] = phy; > + hpriv->nports = 1; > + } else { > + rc = PTR_ERR(phy); > + switch (rc) { > + case -ENOSYS: > + /* No PHY support. Check if PHY is required. */ > + if (of_find_property(dev->of_node, "phys", NULL)) { > + dev_err(dev, "couldn't get sata-phy: ENOSYS\n"); > + goto err_out; > + } > + case -ENODEV: > + /* continue normally */ > + hpriv->phys = NULL; > + break; > + > + default: > + goto err_out; > + > + } > } > } > > @@ -291,7 +403,7 @@ EXPORT_SYMBOL_GPL(ahci_platform_get_resources); > * @host_flags: ahci host flags used in ahci_host_priv > * > * This function does all the usual steps needed to bring up an > - * ahci-platform host, note any necessary resources (ie clks, phy, etc.) > + * ahci-platform host, note any necessary resources (ie clks, phys, etc.) > * must be initialized / enabled before calling this. > * > * RETURNS: > @@ -395,7 +507,7 @@ static void ahci_host_stop(struct ata_host *host) > * @dev: device pointer for the host > * > * This function does all the usual steps needed to suspend an > - * ahci-platform host, note any necessary resources (ie clks, phy, etc.) > + * ahci-platform host, note any necessary resources (ie clks, phys, etc.) > * must be disabled after calling this. > * > * RETURNS: > @@ -432,7 +544,7 @@ EXPORT_SYMBOL_GPL(ahci_platform_suspend_host); > * @dev: device pointer for the host > * > * This function does all the usual steps needed to resume an ahci-platform > - * host, note any necessary resources (ie clks, phy, etc.) must be > + * host, note any necessary resources (ie clks, phys, etc.) must be > * initialized / enabled before calling this. > * > * RETURNS: >
diff --git a/drivers/ata/ahci.h b/drivers/ata/ahci.h index cb8d58926851..47de53284ad7 100644 --- a/drivers/ata/ahci.h +++ b/drivers/ata/ahci.h @@ -332,7 +332,12 @@ struct ahci_host_priv { bool got_runtime_pm; /* Did we do pm_runtime_get? */ struct clk *clks[AHCI_MAX_CLKS]; /* Optional */ struct regulator *target_pwr; /* Optional */ - struct phy *phy; /* If platform uses phy */ + /* + * If platform uses PHYs. There is a 1:1 relation between the port number and + * the PHY position in this array. + */ + struct phy **phys; + unsigned nports; /* Number of ports */ void *plat_data; /* Other platform data */ /* * Optional ahci_start_engine override, if not set this gets set to the diff --git a/drivers/ata/libahci_platform.c b/drivers/ata/libahci_platform.c index db9b90d876dd..095df56432d9 100644 --- a/drivers/ata/libahci_platform.c +++ b/drivers/ata/libahci_platform.c @@ -39,6 +39,67 @@ static struct scsi_host_template ahci_platform_sht = { }; /** + * ahci_platform_enable_phys - Enable PHYs + * @hpriv: host private area to store config values + * + * This function enables all the PHYs found in hpriv->phys, if any. + * If a PHY fails to be enabled, it disables all the PHYs already + * enabled in reverse order and returns an error. + * + * RETURNS: + * 0 on success otherwise a negative error code + */ +int ahci_platform_enable_phys(struct ahci_host_priv *hpriv) +{ + int rc, i; + + for (i = 0; i < hpriv->nports; i++) { + if (!hpriv->phys[i]) + continue; + + rc = phy_init(hpriv->phys[i]); + if (rc) + goto disable_phys; + + rc = phy_power_on(hpriv->phys[i]); + if (rc) { + phy_exit(hpriv->phys[i]); + goto disable_phys; + } + } + + return 0; + +disable_phys: + while (--i >= 0) { + phy_power_off(hpriv->phys[i]); + phy_exit(hpriv->phys[i]); + } + return rc; +} +EXPORT_SYMBOL_GPL(ahci_platform_enable_phys); + +/** + * ahci_platform_disable_phys - Enable PHYs + * @hpriv: host private area to store config values + * + * This function disables all PHYs found in hpriv->phys. + */ +void ahci_platform_disable_phys(struct ahci_host_priv *hpriv) +{ + int i; + + for (i = 0; i < hpriv->nports; i++) { + if (!hpriv->phys[i]) + continue; + + phy_power_off(hpriv->phys[i]); + phy_exit(hpriv->phys[i]); + } +} +EXPORT_SYMBOL_GPL(ahci_platform_disable_phys); + +/** * ahci_platform_enable_clks - Enable platform clocks * @hpriv: host private area to store config values * @@ -92,7 +153,7 @@ EXPORT_SYMBOL_GPL(ahci_platform_disable_clks); * following order: * 1) Regulator * 2) Clocks (through ahci_platform_enable_clks) - * 3) Phy + * 3) Phys * * If resource enabling fails at any point the previous enabled resources * are disabled in reverse order. @@ -114,17 +175,9 @@ int ahci_platform_enable_resources(struct ahci_host_priv *hpriv) if (rc) goto disable_regulator; - if (hpriv->phy) { - rc = phy_init(hpriv->phy); - if (rc) - goto disable_clks; - - rc = phy_power_on(hpriv->phy); - if (rc) { - phy_exit(hpriv->phy); - goto disable_clks; - } - } + rc = ahci_platform_enable_phys(hpriv); + if (rc) + goto disable_clks; return 0; @@ -144,16 +197,13 @@ EXPORT_SYMBOL_GPL(ahci_platform_enable_resources); * * This function disables all ahci_platform managed resources in the * following order: - * 1) Phy + * 1) Phys * 2) Clocks (through ahci_platform_disable_clks) * 3) Regulator */ void ahci_platform_disable_resources(struct ahci_host_priv *hpriv) { - if (hpriv->phy) { - phy_power_off(hpriv->phy); - phy_exit(hpriv->phy); - } + ahci_platform_disable_phys(hpriv); ahci_platform_disable_clks(hpriv); @@ -187,7 +237,7 @@ static void ahci_platform_put_resources(struct device *dev, void *res) * 2) regulator for controlling the targets power (optional) * 3) 0 - AHCI_MAX_CLKS clocks, as specified in the devs devicetree node, * or for non devicetree enabled platforms a single clock - * 4) phy (optional) + * 4) phys (optional) * * RETURNS: * The allocated ahci_host_priv on success, otherwise an ERR_PTR value @@ -197,7 +247,9 @@ struct ahci_host_priv *ahci_platform_get_resources(struct platform_device *pdev) struct device *dev = &pdev->dev; struct ahci_host_priv *hpriv; struct clk *clk; - int i, rc = -ENOMEM; + struct device_node *child; + int i, enabled_ports = 0, rc = -ENOMEM; + u32 mask_port_map = 0; if (!devres_open_group(dev, NULL, GFP_KERNEL)) return ERR_PTR(-ENOMEM); @@ -246,27 +298,87 @@ struct ahci_host_priv *ahci_platform_get_resources(struct platform_device *pdev) hpriv->clks[i] = clk; } - hpriv->phy = devm_phy_get(dev, "sata-phy"); - if (IS_ERR(hpriv->phy)) { - rc = PTR_ERR(hpriv->phy); - switch (rc) { - case -ENOSYS: - /* No PHY support. Check if PHY is required. */ - if (of_find_property(dev->of_node, "phys", NULL)) { - dev_err(dev, "couldn't get sata-phy: ENOSYS\n"); + hpriv->nports = of_get_child_count(dev->of_node); + + if (hpriv->nports) { + hpriv->phys = devm_kzalloc(dev, + hpriv->nports * sizeof(*hpriv->phys), + GFP_KERNEL); + if (!hpriv->phys) { + rc = -ENOMEM; + goto err_out; + } + + for_each_child_of_node(dev->of_node, child) { + u32 port; + + if (!of_device_is_available(child)) + continue; + + if (of_property_read_u32(child, "reg", &port)) { + rc = -EINVAL; goto err_out; } - case -ENODEV: - /* continue normally */ - hpriv->phy = NULL; - break; - case -EPROBE_DEFER: - goto err_out; + if (port >= hpriv->nports) { + dev_warn(dev, "invalid port number %d\n", port); + continue; + } - default: - dev_err(dev, "couldn't get sata-phy\n"); - goto err_out; + mask_port_map |= BIT(port); + + hpriv->phys[port] = devm_of_phy_get(dev, child, NULL); + if (IS_ERR(hpriv->phys[port])) { + rc = PTR_ERR(hpriv->phys[port]); + dev_err(dev, + "couldn't get PHY in node %s: %d\n", + child->name, rc); + goto err_out; + } + + enabled_ports++; + } + if (!enabled_ports) { + dev_warn(dev, "No port enabled\n"); + return ERR_PTR(-ENODEV); + } + + if (!hpriv->mask_port_map) + hpriv->mask_port_map = mask_port_map; + } else { + /* + * If no sub-node was found, keep this for device tree + * compatibility + */ + struct phy *phy = devm_phy_get(dev, "sata-phy"); + if (!IS_ERR(phy)) { + hpriv->phys = devm_kzalloc(dev, sizeof(*hpriv->phys), + GFP_KERNEL); + if (!hpriv->phys) { + rc = -ENOMEM; + goto err_out; + } + + hpriv->phys[0] = phy; + hpriv->nports = 1; + } else { + rc = PTR_ERR(phy); + switch (rc) { + case -ENOSYS: + /* No PHY support. Check if PHY is required. */ + if (of_find_property(dev->of_node, "phys", NULL)) { + dev_err(dev, "couldn't get sata-phy: ENOSYS\n"); + goto err_out; + } + case -ENODEV: + /* continue normally */ + hpriv->phys = NULL; + break; + + default: + goto err_out; + + } } } @@ -291,7 +403,7 @@ EXPORT_SYMBOL_GPL(ahci_platform_get_resources); * @host_flags: ahci host flags used in ahci_host_priv * * This function does all the usual steps needed to bring up an - * ahci-platform host, note any necessary resources (ie clks, phy, etc.) + * ahci-platform host, note any necessary resources (ie clks, phys, etc.) * must be initialized / enabled before calling this. * * RETURNS: @@ -395,7 +507,7 @@ static void ahci_host_stop(struct ata_host *host) * @dev: device pointer for the host * * This function does all the usual steps needed to suspend an - * ahci-platform host, note any necessary resources (ie clks, phy, etc.) + * ahci-platform host, note any necessary resources (ie clks, phys, etc.) * must be disabled after calling this. * * RETURNS: @@ -432,7 +544,7 @@ EXPORT_SYMBOL_GPL(ahci_platform_suspend_host); * @dev: device pointer for the host * * This function does all the usual steps needed to resume an ahci-platform - * host, note any necessary resources (ie clks, phy, etc.) must be + * host, note any necessary resources (ie clks, phys, etc.) must be * initialized / enabled before calling this. * * RETURNS:
The current implementation of the libahci does not allow to use multiple PHYs. This patch adds the support of multiple PHYs by the libahci while keeping the old bindings valid for device tree compatibility. This introduce a new way of defining SATA ports in the device tree, with one port per sub-node. This as the advantage of allowing a per port configuration. Because some ports may be accessible but disabled in the device tree, the port_map mask is computed automatically when using this. Signed-off-by: Antoine Ténart <antoine.tenart@free-electrons.com> --- drivers/ata/ahci.h | 7 +- drivers/ata/libahci_platform.c | 190 ++++++++++++++++++++++++++++++++--------- 2 files changed, 157 insertions(+), 40 deletions(-)