ppc/sata-fsl: orphan config value: CONFIG_MPC8315_DS

[Scott Wood]

On 04/30/2013 01:41:49 AM, Anthony Foiani wrote:
> Apologies for resurrecting a very old thread, but...
> 
> On 05/30/2012 02:14 PM, Anthony Foiani wrote:
>> 
>> Maybe someone who knows devtree really well could crank that out in a
>> few minutes... but I'm not that person.  :)
> Well, I wasn't last year, but this year I decided that I didn't  
> care.  Took me about an hour, not a minute, but...
> 
> Having been bitten by this config symbol disappearing one more time,  
> please find attached my attempt at using information out of the  
> device tree to enable this hack.
> 
> Patch is against 3.4.36 or so; hopefully upstream hasn't diverged  
> very much.
> 
> Tested by me, so feel free to add that tag if required.
> 
> Thanks,
> Anthony Foiani

------quoted attachment  
"0001-sata-fsl-allow-device-tree-to-limit-sata-speed.patch"------
> >From c0a85758a669b430c0a6af825e71d18a54ef88d0 Mon Sep 17 00:00:00  
> 2001
> From: Anthony Foiani <anthony.foiani at gmail.com>
> Date: Mon, 29 Apr 2013 23:44:14 -0600
> Subject: [PATCH] sata: fsl: allow device tree to limit sata speed.
> 
> There used to be an "orphan" config symbol (CONFIG_MPC8315_DS) that
> would artificially limit SATA speed to generation 1 (1.5Gbps).
> 
> Since that config symbol got lost whenever any sort of configuration
> was done, we instead extract the limitation from the device tree.
> 
> Signed-off-by: Anthony Foiani <anthony.foiani at gmail.com>
> ---
>  .../devicetree/bindings/powerpc/fsl/board.txt      | 23 +++++++++++
>  drivers/ata/sata_fsl.c                             | 44  
> ++++++++++++++++++----
>  2 files changed, 59 insertions(+), 8 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/powerpc/fsl/board.txt  
> b/Documentation/devicetree/bindings/powerpc/fsl/board.txt
> index 380914e..6a30398 100644
> --- a/Documentation/devicetree/bindings/powerpc/fsl/board.txt
> +++ b/Documentation/devicetree/bindings/powerpc/fsl/board.txt
> @@ -67,3 +67,26 @@ Example:
>  			gpio-controller;
>  		};
>  	};
> +
> +* Maximum SATA Generation workaround
> +
> +Some boards advertise SATA speeds that they cannot actually achieve.
> +Previously, this was dealt with via the orphaned config symbol
> +CONFIG_MPC8315_DS.  We now have a device tree property
> +"fsl,sata-max-gen" to control this.  It should live within the "sata"
> +block.
> +
> +Example:
> +
> +		sata at 18000 {
> +			compatible = "fsl,mpc8315-sata", "fsl,pq-sata";
> +			reg = <0x18000 0x1000>;
> +			cell-index = <1>;
> +			interrupts = <44 0x8>;
> +			interrupt-parent = <&ipic>;
> +			fsl,sata-max-gen = <1>;
> +		};
> +

This might be OK for a new board, but adding it now means that people  
using existing device trees won't get the workaround.  It might be  
better to just put the knowledge in platform code.

> +By default, there is no limitation; if a value is given, it indicates
> +the maximum "generation" that should be negotiated.  Gen 1 is  
> 1.5Gbps,
> +Gen 2 is 3.0Gbps.
> diff --git a/drivers/ata/sata_fsl.c b/drivers/ata/sata_fsl.c
> index d6577b9..6d3ec47 100644
> --- a/drivers/ata/sata_fsl.c
> +++ b/drivers/ata/sata_fsl.c
> @@ -274,6 +274,17 @@ struct sata_fsl_port_priv {
>  };
> 
>  /*
> + * speed negotiation.
> + */
> +
> +enum {
> +	SCR_SPEED_NEG_MASK	= 0xf0,
> +	SCR_SPEED_NEG_UNLIMITED	= 0x00,
> +	SCR_SPEED_NEG_GEN_1	= 0x10, /* 1.5Gbps max */
> +	SCR_SPEED_NEG_GEN_2	= 0x20  /* 3.0Gbps max */
> +};
> +
> +/*
>   * ata_port->host_set private data
>   */
>  struct sata_fsl_host_priv {
> @@ -282,6 +293,7 @@ struct sata_fsl_host_priv {
>  	void __iomem *csr_base;
>  	int irq;
>  	int data_snoop;
> +	u32 speed_neg;
>  	struct device_attribute intr_coalescing;
>  };
> 
> @@ -726,19 +738,23 @@ static int sata_fsl_port_start(struct ata_port  
> *ap)
>  	VPRINTK("HControl = 0x%x\n", ioread32(hcr_base + HCONTROL));
>  	VPRINTK("CHBA  = 0x%x\n", ioread32(hcr_base + CHBA));
> 
> -#ifdef CONFIG_MPC8315_DS
>  	/*
>  	 * Workaround for 8315DS board 3gbps link-up issue,
>  	 * currently limit SATA port to GEN1 speed
>  	 */
> -	sata_fsl_scr_read(&ap->link, SCR_CONTROL, &temp);
> -	temp &= ~(0xF << 4);
> -	temp |= (0x1 << 4);
> -	sata_fsl_scr_write(&ap->link, SCR_CONTROL, temp);
> +	if ( host_priv->speed_neg != SCR_SPEED_NEG_UNLIMITED )
> +	{
> 
> -	sata_fsl_scr_read(&ap->link, SCR_CONTROL, &temp);
> -	dev_warn(dev, "scr_control, speed limited to %x\n", temp);
> -#endif
> +		u32 orig;
> +		sata_fsl_scr_read(&ap->link, SCR_CONTROL, &orig);
> +		temp = ( ( orig                 & ~SCR_SPEED_NEG_MASK )  
> |
> +			 ( host_priv->speed_neg &  SCR_SPEED_NEG_MASK )  
> );
> +		sata_fsl_scr_write(&ap->link, SCR_CONTROL, temp);
> +
> +		sata_fsl_scr_read(&ap->link, SCR_CONTROL, &temp);
> +		dev_warn(dev, "speed limited, scr_control 0x%x ->  
> 0x%x\n",
> +			 orig, temp);
> +	}
> 
>  	return 0;
>  }
> @@ -1437,6 +1453,18 @@ static int sata_fsl_probe(struct  
> platform_device *ofdev)
>  	else
>  		host_priv->data_snoop = DATA_SNOOP_ENABLE_V1;
> 
> +	if (!of_property_read_u32(ofdev->dev.of_node,  
> "fsl,sata-max-gen",
> +				  &temp))
> +	{
> +		switch (temp)
> +		{
> +		case 1: host_priv->speed_neg = SCR_SPEED_NEG_GEN_1;  
> break;
> +		case 2: host_priv->speed_neg = SCR_SPEED_NEG_GEN_2;  
> break;
> +		}
> +		dev_warn(&ofdev->dev, "speed limit set to gen %u  
> (0x%x)\n",
> +			 temp, host_priv->speed_neg);
> +	}
> +
>  	/* allocate host structure */
>  	host = ata_host_alloc_pinfo(&ofdev->dev, ppi,  
> SATA_FSL_MAX_PORTS);
>  	if (!host) {

Please use standard Linux coding style, and submit the patch inline  
rather than as an attachment (e.g. use git send-email).

-Scott