On Fri, Nov 13, 2015 at 04:02:23PM +0530, Anurag Kumar Vulisha wrote:
> In SATA, speed negotiation happens with  OOB(Out of Band) signals. These OOB
> signal timing values are configured through vendor specific registers in the
> SATA controller. These OOB timings depends on the generator and detector clock
> frequency, which varies from board to board (ex: ep108, zcu102 and zc1751 has
> different clock frequencies).

Could you calculate the timings based on the frequency instead? 

> Since to make ahci_ceva driver generic, it would be better to move these
> settings to the device-tree node and read them from driver.
> 
> This patch does the same.
> 
> Signed-off-by: Anurag Kumar Vulisha <anura...@xilinx.com>
> ---
>  .../devicetree/bindings/ata/ahci-ceva.txt          |   38 +++++++++
>  drivers/ata/ahci_ceva.c                            |   84 
> ++++++++++++++------
>  2 files changed, 99 insertions(+), 23 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/ata/ahci-ceva.txt 
> b/Documentation/devicetree/bindings/ata/ahci-ceva.txt
> index 7ca8b97..66fcd10 100644
> --- a/Documentation/devicetree/bindings/ata/ahci-ceva.txt
> +++ b/Documentation/devicetree/bindings/ata/ahci-ceva.txt
> @@ -5,6 +5,36 @@ Required properties:
>    - compatible: Compatibility string. Must be 'ceva,ahci-1v84'.
>    - clocks: Input clock specifier. Refer to common clock bindings.
>    - interrupts: Interrupt specifier. Refer to interrupt binding.
> +    - ceva,p0-cominit-params: OOB timing value for COMINIT parameter for 
> port 0.
> +    - ceva,p1-cominit-params: OOB timing value for COMINIT parameter for 
> port 1.

This doesn't really scale when you have more than 2 ports. Given that 
you know the length for each port, just make it a single property (i.e. 
an array of ports).

> +                     The fields for the above parameter must be as shown 
> below:
> +                     ceva,phy-cominit-params = /bits/ 8 <CIBGMN CIBGMX CIBGN 
> CINMP>;
> +                     CINMP : COMINIT Negate Minimum Period.
> +                     CIBGN : COMINIT Burst Gap Nominal.
> +                     CIBGMX: COMINIT Burst Gap Maximum.
> +                     CIBGMN: COMINIT Burst Gap Minimum.
> +    - ceva,p0-comwake-params: OOB timing value for COMWAKE parameter for 
> port 0.
> +    - ceva,p1-comwake-params: OOB timing value for COMWAKE parameter for 
> port 1.
> +                     The fields for the above parameter must be as shown 
> below:
> +                     ceva,phy-comwake-params = /bits/ 8 <CWBGMN CWBGMX CWBGN 
> CWNMP>;
> +                     CWBGMN: COMWAKE Burst Gap Minimum.
> +                     CWBGMX: COMWAKE Burst Gap Maximum.
> +                     CWBGN: COMWAKE Burst Gap Nominal.
> +                     CWNMP: COMWAKE Negate Minimum Period.
> +    - ceva,p0-burst-params: Burst timing value for COM parameter for port 0.
> +    - ceva,p1-burst-params: Burst timing value for COM parameter for port 1.
> +                     The fields for the above parameter must be as shown 
> below:
> +                     ceva,phy-burst-params = /bits/ 8 <BMX BNM SFD PTST>;
> +                     BMX: COM Burst Maximum.
> +                     BNM: COM Burst Nominal.
> +                     SFD: Signal Failure Detection value.
> +                     PTST: Partial to Slumber timer value.
> +    - ceva,p0-retry-params: Retry interval timing value for port 0.
> +    - ceva,p1-retry-params: Retry interval timing value for port 1.
> +                     The fields for the above parameter must be as shown 
> below:
> +                     ceva,phy-retry-params = /bits/ 16 <RIT RCT>;
> +                     RIT:  Retry Interval Timer.
> +                     RCT:  Rate Change Timer.
>  
>  Optional properties:
>    - ceva,broken-gen2: limit to gen1 speed instead of gen2.
> @@ -17,4 +47,12 @@ Examples:
>               interrupts = <0 133 4>;
>               clocks = <&clkc SATA_CLK_ID>;
>               ceva,broken-gen2;
> +             ceva,p0-cominit-params = /bits/ 8 <0x0F 0x25 0x18 0x29>;
> +             ceva,p0-comwake-params = /bits/ 8 <0x04 0x0B 0x08 0x0F>;
> +             ceva,p0-burst-params = /bits/ 8 <0x0A 0x08 0x4A 0x06>;
> +             ceva,p0-retry-params = /bits/ 16 <0x0216 0x7F06>;
> +             ceva,p1-cominit-params = /bits/ 8 <0x0F 0x25 0x18 0x29>;
> +             ceva,p1-comwake-params = /bits/ 8 <0x04 0x0B 0x08 0x0F>;
> +             ceva,p1-burst-params = /bits/ 8 <0x0A 0x08 0x4A 0x06>;
> +             ceva,p1-retry-params = /bits/ 16 <0x0216 0x7F06>;
>       };
> diff --git a/drivers/ata/ahci_ceva.c b/drivers/ata/ahci_ceva.c
> index 207649d..59de2ca 100644
> --- a/drivers/ata/ahci_ceva.c
> +++ b/drivers/ata/ahci_ceva.c
> @@ -50,21 +50,6 @@
>  #define PPCFG_PSS_EN (1 << 29)
>  #define PPCFG_ESDF_EN        (1 << 31)
>  
> -#define PP2C_CIBGMN  0x0F
> -#define PP2C_CIBGMX  (0x25 << 8)
> -#define PP2C_CIBGN   (0x18 << 16)
> -#define PP2C_CINMP   (0x29 << 24)
> -
> -#define PP3C_CWBGMN  0x04
> -#define PP3C_CWBGMX  (0x0B << 8)
> -#define PP3C_CWBGN   (0x08 << 16)
> -#define PP3C_CWNMP   (0x0F << 24)
> -
> -#define PP4C_BMX     0x0a
> -#define PP4C_BNM     (0x08 << 8)
> -#define PP4C_SFD     (0x4a << 16)
> -#define PP4C_PTST    (0x06 << 24)
> -
>  #define PP5C_RIT     0x60216
>  #define PP5C_RCT     (0x7f0 << 20)
>  
> @@ -87,6 +72,11 @@
>  
>  struct ceva_ahci_priv {
>       struct platform_device *ahci_pdev;
> +     /* Port Phy2Cfg Register */
> +     u32 pp2c[NR_PORTS];
> +     u32 pp3c[NR_PORTS];
> +     u32 pp4c[NR_PORTS];
> +     u32 pp5c[NR_PORTS];
>       int flags;
>  };
>  
> @@ -131,20 +121,16 @@ static void ahci_ceva_setup(struct ahci_host_priv 
> *hpriv)
>               writel(tmp, mmio + AHCI_VEND_PPCFG);
>  
>               /* Phy Control OOB timing parameters COMINIT */
> -             tmp = PP2C_CIBGMN | PP2C_CIBGMX | PP2C_CIBGN | PP2C_CINMP;
> -             writel(tmp, mmio + AHCI_VEND_PP2C);
> +             writel(cevapriv->pp2c[i], mmio + AHCI_VEND_PP2C);
>  
>               /* Phy Control OOB timing parameters COMWAKE */
> -             tmp = PP3C_CWBGMN | PP3C_CWBGMX | PP3C_CWBGN | PP3C_CWNMP;
> -             writel(tmp, mmio + AHCI_VEND_PP3C);
> +             writel(cevapriv->pp3c[i], mmio + AHCI_VEND_PP3C);
>  
>               /* Phy Control Burst timing setting */
> -             tmp = PP4C_BMX | PP4C_BNM | PP4C_SFD | PP4C_PTST;
> -             writel(tmp, mmio + AHCI_VEND_PP4C);
> +             writel(cevapriv->pp4c[i], mmio + AHCI_VEND_PP4C);
>  
>               /* Rate Change Timer and Retry Interval Timer setting */
> -             tmp = PP5C_RIT | PP5C_RCT;
> -             writel(tmp, mmio + AHCI_VEND_PP5C);
> +             writel(cevapriv->pp5c[i], mmio + AHCI_VEND_PP5C);
>  
>               /* Rx Watermark setting  */
>               tmp = PTC_RX_WM_VAL | PTC_RSVD;
> @@ -187,6 +173,58 @@ static int ceva_ahci_probe(struct platform_device *pdev)
>       if (of_property_read_bool(np, "ceva,broken-gen2"))
>               cevapriv->flags = CEVA_FLAG_BROKEN_GEN2;
>  
> +     /* Read OOB timing value for COMINIT from device-tree */
> +     if (of_property_read_u8_array(np, "ceva,p0-cominit-params",
> +                                     (u8 *)&cevapriv->pp2c[0], 4) < 0) {
> +             dev_warn(dev, "ceva,p0-cominit-params property not defined\n");

I'd really like to move all the error prints into the of_property_* 
functions instead, but then we'd probably need *_optional variants to 
avoid spewing to the console.

Rob

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Reply via email to