From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751286AbdAMUgV (ORCPT ); Fri, 13 Jan 2017 15:36:21 -0500 Received: from vern.gendns.com ([206.190.152.46]:49628 "EHLO vern.gendns.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751031AbdAMUgM (ORCPT ); Fri, 13 Jan 2017 15:36:12 -0500 Subject: Re: [PATCH 07/10] sata: ahci_da850: add support for the da850,clk_multiplier DT property To: Bartosz Golaszewski , Kevin Hilman , Sekhar Nori , Patrick Titiano , Michael Turquette , Tejun Heo , Rob Herring , Mark Rutland , Russell King References: <1484311084-31547-1-git-send-email-bgolaszewski@baylibre.com> <1484311084-31547-8-git-send-email-bgolaszewski@baylibre.com> Cc: linux-ide@vger.kernel.org, devicetree@vger.kernel.org, linux-kernel@vger.kernel.org, linux-arm-kernel@lists.infradead.org From: David Lechner Message-ID: <357b3929-cf5c-43ce-753e-03ccd7583f18@lechnology.com> Date: Fri, 13 Jan 2017 13:29:36 -0600 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:45.0) Gecko/20100101 Thunderbird/45.5.1 MIME-Version: 1.0 In-Reply-To: <1484311084-31547-8-git-send-email-bgolaszewski@baylibre.com> Content-Type: text/plain; charset=windows-1252; format=flowed Content-Transfer-Encoding: 7bit X-AntiAbuse: This header was added to track abuse, please include it with any abuse report X-AntiAbuse: Primary Hostname - vern.gendns.com X-AntiAbuse: Original Domain - vger.kernel.org X-AntiAbuse: Originator/Caller UID/GID - [47 12] / [47 12] X-AntiAbuse: Sender Address Domain - lechnology.com X-Get-Message-Sender-Via: vern.gendns.com: authenticated_id: davidmain+lechnology.com/only user confirmed/virtual account not confirmed X-Authenticated-Sender: vern.gendns.com: davidmain@lechnology.com X-Source: X-Source-Args: X-Source-Dir: Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On 01/13/2017 06:38 AM, Bartosz Golaszewski wrote: > Currently the clock multiplier is hardcoded in the driver for > the da850-evm board. Make it configurable over DT, but keep the > previous value as default in case the property is missing. > > Signed-off-by: Bartosz Golaszewski > --- > drivers/ata/ahci_da850.c | 83 +++++++++++++++++++++++++++++++++++++++++++----- > 1 file changed, 75 insertions(+), 8 deletions(-) > > diff --git a/drivers/ata/ahci_da850.c b/drivers/ata/ahci_da850.c > index bb9eb4c..cd04caf 100644 > --- a/drivers/ata/ahci_da850.c > +++ b/drivers/ata/ahci_da850.c > @@ -28,17 +28,70 @@ > #define SATA_PHY_TXSWING(x) ((x) << 19) > #define SATA_PHY_ENPLL(x) ((x) << 31) > > +struct da850_sata_mpy_mapping { > + unsigned int multiplier; > + unsigned int regval; > +}; > + > +static const struct da850_sata_mpy_mapping da850_sata_mpy_table[] = { > + { > + .multiplier = 5, > + .regval = 0x01, > + }, > + { > + .multiplier = 6, > + .regval = 0x02, > + }, > + { > + .multiplier = 8, > + .regval = 0x04, > + }, > + { > + .multiplier = 10, > + .regval = 0x05, > + }, > + { > + .multiplier = 12, > + .regval = 0x06, > + }, > + /* TODO Add 12.5 multiplier. */ Looks like you should be using an enum here for the multiplier field. > + { > + .multiplier = 15, > + .regval = 0x08, > + }, > + { > + .multiplier = 20, > + .regval = 0x09, > + }, > + { > + .multiplier = 25, > + .regval = 0x0a, > + } > +}; > + > +static const struct da850_sata_mpy_mapping * > +da850_sata_get_mpy(unsigned int multiplier) > +{ > + int i; > + > + for (i = 0; i < ARRAY_SIZE(da850_sata_mpy_table); i++) > + if (da850_sata_mpy_table[i].multiplier == multiplier) > + return &da850_sata_mpy_table[i]; > + > + return NULL; > +} > + > /* > * The multiplier needed for 1.5GHz PLL output. > * > - * NOTE: This is currently hardcoded to be suitable for 100MHz crystal > - * frequency (which is used by DA850 EVM board) and may need to be changed > - * if you would like to use this driver on some other board. > + * This is the default value suitable for the 100MHz crystal frequency > + * used by DA850 EVM board, which doesn't use DT. As I said in a reply on another patch, it seems like it would be better to use a clock that represents the crystal and use clk_get_rate() and calculate the multiplier from that. For example, we have done something like this in usb20_phy_clk_set_parent() in arch/arm/mach-davinci/usb-da8xx.c. > */ > -#define DA850_SATA_CLK_MULTIPLIER 7 > +#define DA850_SATA_CLK_MULTIPLIER_DEFAULT 15 > > static void da850_sata_init(struct device *dev, void __iomem *pwrdn_reg, > - void __iomem *ahci_base) > + void __iomem *ahci_base, > + const struct da850_sata_mpy_mapping *mpy) > { > unsigned int val; > > @@ -47,7 +100,7 @@ static void da850_sata_init(struct device *dev, void __iomem *pwrdn_reg, > val &= ~BIT(0); > writel(val, pwrdn_reg); > > - val = SATA_PHY_MPY(DA850_SATA_CLK_MULTIPLIER + 1) | SATA_PHY_LOS(1) | > + val = SATA_PHY_MPY(mpy->regval) | SATA_PHY_LOS(1) | > SATA_PHY_RXCDR(4) | SATA_PHY_RXEQ(1) | SATA_PHY_TXSWING(3) | > SATA_PHY_ENPLL(1); > > @@ -87,10 +140,12 @@ static struct scsi_host_template ahci_platform_sht = { > > static int ahci_da850_probe(struct platform_device *pdev) > { > + const struct da850_sata_mpy_mapping *mpy; > struct device *dev = &pdev->dev; > struct ahci_host_priv *hpriv; > - struct resource *res; > + unsigned int multiplier; > void __iomem *pwrdn_reg; > + struct resource *res; > int rc; > > hpriv = ahci_platform_get_resources(pdev); > @@ -109,7 +164,19 @@ static int ahci_da850_probe(struct platform_device *pdev) > if (!pwrdn_reg) > goto disable_resources; > > - da850_sata_init(dev, pwrdn_reg, hpriv->mmio); > + rc = of_property_read_u32(dev->of_node, > + "da850,clk_multiplier", &multiplier); > + if (rc) > + multiplier = DA850_SATA_CLK_MULTIPLIER_DEFAULT; > + > + mpy = da850_sata_get_mpy(multiplier); > + if (!mpy) { > + dev_err(dev, "invalid multiplier value: %u\n", multiplier); > + rc = -EINVAL; > + goto disable_resources; > + } > + > + da850_sata_init(dev, pwrdn_reg, hpriv->mmio, mpy); > > rc = ahci_platform_init_host(pdev, hpriv, &ahci_da850_port_info, > &ahci_platform_sht); >