Linux ATA/IDE development
 help / color / mirror / Atom feed
* Re: [PATCH] Revert "ata: sata_mv: Convert to devm_ioremap_resource()"
From: Andy Shevchenko @ 2017-05-25 12:53 UTC (permalink / raw)
  To: Andrew Lunn; +Cc: Gregory CLEMENT, Tejun Heo, Jason Cooper, linux-ide
In-Reply-To: <20170524142956.GE26577@lunn.ch>

On Wed, 2017-05-24 at 16:29 +0200, Andrew Lunn wrote:
> On Wed, May 24, 2017 at 05:00:04PM +0300, Andy Shevchenko wrote:
> > On Wed, 2017-05-24 at 15:41 +0200, Andrew Lunn wrote:
> > > > I'm wondering where exactly first resource acquiring is
> > > > happening.
> > > 
> > > drivers/phy/phy-mvebu-sata.c
> > > 
> > 
> > In which tree? I have nothing like this in linux-next.
> 
> git log drivers/phy/phy-mvebu-sata.c
> 
> It has been there since Tue Dec 17 21:21:50 2013.
> 
> > I'm on the side that revert just hides the real issue back.
> 
> What issue? You are allowed to have overlapping memory resources.

Exactly, and then the question why did you do a revert?

AFAIU the problem is that platform core tries to reserve the resource
and you can't do it twice for overlapping region (see insert_resource()
call implementation).

Since it's allowed and we have more devices which requires that
(basically that ones that have phy embedded in common address space) we
might reconsider the policy used in platform core.

-- 
Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Intel Finland Oy

^ permalink raw reply

* RE: [PATCH V3 1/3] ata: ahci_tegra: Add AHCI support for tegra210
From: Preetham Chandru @ 2017-05-25 11:15 UTC (permalink / raw)
  To: Mikko Perttunen,
	thierry.reding-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org,
	tj-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org
  Cc: linux-ide-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	Laxman Dewangan,
	preetham260-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org,
	Venu Byravarasu, Pavan Kunapuli,
	linux-tegra-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
In-Reply-To: <e7a2cbef-71c7-cffb-58d5-aac998c8e156-/1wQRMveznE@public.gmane.org>



>-----Original Message-----
>From: Mikko Perttunen [mailto:cyndis-/1wQRMveznE@public.gmane.org]
>Sent: Wednesday, May 24, 2017 12:46 PM
>To: Preetham Chandru <pchandru-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>; thierry.reding-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org;
>tj-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org
>Cc: linux-ide-u79uwXL29TY76Z2rM5mHXA@public.gmane.org; Laxman Dewangan <ldewangan-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>;
>preetham260-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org; Venu Byravarasu <vbyravarasu-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>; Pavan
>Kunapuli <pkunapuli-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>; linux-tegra-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
>Subject: Re: [PATCH V3 1/3] ata: ahci_tegra: Add AHCI support for tegra210
>
>Also Vagrant Cascadian mentioned on IRC that the call to
>tegra_ahci_handle_quirks is currently before the definition of the function which
>causes warnings or compilation errors. So please add a declaration of the
>function before the call.
>

Okay.


>Cheers,
>Mikko.
>
>On 24.05.2017 10:14, Mikko Perttunen wrote:
>> On 24.05.2017 08:44, Preetham Chandru wrote:
>>> ...

^ permalink raw reply

* RE: [PATCH V3 1/3] ata: ahci_tegra: Add AHCI support for tegra210
From: Preetham Chandru @ 2017-05-25 11:11 UTC (permalink / raw)
  To: Mikko Perttunen,
	thierry.reding-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org,
	tj-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org
  Cc: linux-ide-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	Laxman Dewangan,
	preetham260-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org,
	Venu Byravarasu, Pavan Kunapuli,
	linux-tegra-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
In-Reply-To: <6a39df12-c51d-d5b5-c126-1f264bb00ad0-/1wQRMveznE@public.gmane.org>

>>>>> +		ahci_tegra_port_info.flags |= ATA_FLAG_NO_DIPM;
>>>
>>> Looking at the downstream device trees, it looks like DIPM and DEVSLP
>>> are still disabled for Tegra186 - so why don't we just hardcode these
>>> quirks, by always writing the MISC_CNTL_1_0 register and just adding
>>> ATA_FLAG_NO_DIPM to ahci_tegra_port_info's static definition.
>>>
>>
>> For Tegra186 the devslp pin is shared with PCIE clk req pin.
>> In downstream PCIe driver by default assumes that CLKREQ is present and it
>owns it.
>> So by default devslp is disabled. We verify devslp by making sure that
>> pcie does not own this pin. This issue will be fixed in future chips.
>
>Ok. How about DIPM, can we hardcode that? If not, or if it is fixed in Xavier, I
>think it would be cleaner to have two static port_info structs and select between
>then based on the quirk, so that we don't need to mutate the static data.
>

No, DIPM is not fixed in that chip as well. I will hardcode it now and in future when it gets fixed we can have two port_info structs as suggested.

^ permalink raw reply

* Re: [PATCH] Revert "ata: sata_mv: Convert to devm_ioremap_resource()"
From: Tejun Heo @ 2017-05-24 15:07 UTC (permalink / raw)
  To: Andrew Lunn; +Cc: Andy Shevchenko, Gregory Clement, Jason Cooper, linux-ide
In-Reply-To: <1495582775-3717-1-git-send-email-andrew@lunn.ch>

On Wed, May 24, 2017 at 01:39:35AM +0200, Andrew Lunn wrote:
> This reverts commit 368e5fbdfc60732643f34f538823ed4bc8829827.
> 
> devm_ioremap_resource() enforces that there are no overlapping
> resources, where as devm_ioremap() does not. The sata phy driver needs
> a subset of the sata IO address space, so maps some of the sata
> address space. As a result, sata_mv now fails to probe, reporting it
> cannot get its resources, and so we don't have any SATA disks.
> 
> Signed-off-by: Andrew Lunn <andrew@lunn.ch>

Applied to libata/v4.12-fixes.

Thanks.

-- 
tejun

^ permalink raw reply

* Re: [PATCH] Revert "ata: sata_mv: Convert to devm_ioremap_resource()"
From: Andrew Lunn @ 2017-05-24 14:29 UTC (permalink / raw)
  To: Andy Shevchenko; +Cc: Gregory CLEMENT, Tejun Heo, Jason Cooper, linux-ide
In-Reply-To: <1495634404.6967.109.camel@linux.intel.com>

On Wed, May 24, 2017 at 05:00:04PM +0300, Andy Shevchenko wrote:
> On Wed, 2017-05-24 at 15:41 +0200, Andrew Lunn wrote:
> > > I'm wondering where exactly first resource acquiring is happening.
> > 
> > drivers/phy/phy-mvebu-sata.c
> > 
> 
> In which tree? I have nothing like this in linux-next.

git log drivers/phy/phy-mvebu-sata.c

It has been there since Tue Dec 17 21:21:50 2013.

> I'm on the side that revert just hides the real issue back.

What issue? You are allowed to have overlapping memory resources.

     Andrew

^ permalink raw reply

* Re: [PATCH] Revert "ata: sata_mv: Convert to devm_ioremap_resource()"
From: Gregory CLEMENT @ 2017-05-24 14:11 UTC (permalink / raw)
  To: Andy Shevchenko; +Cc: Andrew Lunn, Tejun Heo, Jason Cooper, linux-ide
In-Reply-To: <1495634404.6967.109.camel@linux.intel.com>

Hi Andy,
 
 On mer., mai 24 2017, Andy Shevchenko <andriy.shevchenko@linux.intel.com> wrote:

> On Wed, 2017-05-24 at 15:41 +0200, Andrew Lunn wrote:
>> > I'm wondering where exactly first resource acquiring is happening.
>> 
>> drivers/phy/phy-mvebu-sata.c
>> 
>
> In which tree? I have nothing like this in linux-next.

in v4.12-rc2 it still exist:
http://elixir.free-electrons.com/linux/v4.12-rc2/source/drivers/phy/phy-mvebu-sata.c

Gregory

>
> I'm on the side that revert just hides the real issue back.
>
> So, it looks like better to remove exclusivity from platform core
> instead.
>
> -- 
> Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> Intel Finland Oy

-- 
Gregory Clement, Free Electrons
Kernel, drivers, real-time and embedded Linux
development, consulting, training and support.
http://free-electrons.com

^ permalink raw reply

* Re: [PATCH] Revert "ata: sata_mv: Convert to devm_ioremap_resource()"
From: Andy Shevchenko @ 2017-05-24 14:00 UTC (permalink / raw)
  To: Andrew Lunn; +Cc: Gregory CLEMENT, Tejun Heo, Jason Cooper, linux-ide
In-Reply-To: <20170524134127.GB26577@lunn.ch>

On Wed, 2017-05-24 at 15:41 +0200, Andrew Lunn wrote:
> > I'm wondering where exactly first resource acquiring is happening.
> 
> drivers/phy/phy-mvebu-sata.c
> 

In which tree? I have nothing like this in linux-next.

I'm on the side that revert just hides the real issue back.

So, it looks like better to remove exclusivity from platform core
instead.

-- 
Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Intel Finland Oy

^ permalink raw reply

* Re: [PATCH] Revert "ata: sata_mv: Convert to devm_ioremap_resource()"
From: Andrew Lunn @ 2017-05-24 13:41 UTC (permalink / raw)
  To: Andy Shevchenko; +Cc: Gregory CLEMENT, Tejun Heo, Jason Cooper, linux-ide
In-Reply-To: <1495632972.6967.104.camel@linux.intel.com>

> I'm wondering where exactly first resource acquiring is happening.

drivers/phy/phy-mvebu-sata.c

	Andrew

^ permalink raw reply

* Re: [PATCH] Revert "ata: sata_mv: Convert to devm_ioremap_resource()"
From: Andy Shevchenko @ 2017-05-24 13:36 UTC (permalink / raw)
  To: Gregory CLEMENT, Andrew Lunn; +Cc: Tejun Heo, Jason Cooper, linux-ide
In-Reply-To: <87d1aywgrj.fsf@free-electrons.com>

On Wed, 2017-05-24 at 15:01 +0200, Gregory CLEMENT wrote:
> Hi Andrew,
>  
>  On mer., mai 24 2017, Andrew Lunn <andrew@lunn.ch> wrote:
> 
> > This reverts commit 368e5fbdfc60732643f34f538823ed4bc8829827.
> > 
> > devm_ioremap_resource() enforces that there are no overlapping
> > resources, where as devm_ioremap() does not. The sata phy driver
> > needs
> > a subset of the sata IO address space, so maps some of the sata
> > address space. As a result, sata_mv now fails to probe, reporting it
> > cannot get its resources, and so we don't have any SATA disks.
> > 
> > Signed-off-by: Andrew Lunn <andrew@lunn.ch>
> 
> For this patch as it fixes a regresssion:
> 
> Acked-by: Gregory CLEMENT <gregory.clement@free-electrons.com>
> 
> However, as the same set of register are accessed by two drivers, is
> there a risk for a race condition ?
> 
> In this case we may consider to use a regmap.
> 
> Or maybe they never aces the same register ?

I'm wondering where exactly first resource acquiring is happening.

For me seems that the initial patch unhides the issue.

> 
> In this case it is safe to let the driver as is, but adding a comment
> could be useful.


-- 
Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Intel Finland Oy

^ permalink raw reply

* Re: [PATCH] Revert "ata: sata_mv: Convert to devm_ioremap_resource()"
From: Andrew Lunn @ 2017-05-24 13:12 UTC (permalink / raw)
  To: Gregory CLEMENT; +Cc: Tejun Heo, Andy Shevchenko, Jason Cooper, linux-ide
In-Reply-To: <87d1aywgrj.fsf@free-electrons.com>

> However, as the same set of register are accessed by two drivers, is
> there a risk for a race condition ?

Hi Gregory

I checked this when i wrote the PHY driver.

phy_power_on() and phy_power_off() are only used in the probe and
remove function. Everything is done synchronously. So there is no
chance of a race.

       Andrew
 

^ permalink raw reply

* Re: [PATCH] Revert "ata: sata_mv: Convert to devm_ioremap_resource()"
From: Gregory CLEMENT @ 2017-05-24 13:01 UTC (permalink / raw)
  To: Andrew Lunn; +Cc: Tejun Heo, Andy Shevchenko, Jason Cooper, linux-ide
In-Reply-To: <1495582775-3717-1-git-send-email-andrew@lunn.ch>

Hi Andrew,
 
 On mer., mai 24 2017, Andrew Lunn <andrew@lunn.ch> wrote:

> This reverts commit 368e5fbdfc60732643f34f538823ed4bc8829827.
>
> devm_ioremap_resource() enforces that there are no overlapping
> resources, where as devm_ioremap() does not. The sata phy driver needs
> a subset of the sata IO address space, so maps some of the sata
> address space. As a result, sata_mv now fails to probe, reporting it
> cannot get its resources, and so we don't have any SATA disks.
>
> Signed-off-by: Andrew Lunn <andrew@lunn.ch>
For this patch as it fixes a regresssion:

Acked-by: Gregory CLEMENT <gregory.clement@free-electrons.com>

However, as the same set of register are accessed by two drivers, is
there a risk for a race condition ?

In this case we may consider to use a regmap.

Or maybe they never aces the same register ?

In this case it is safe to let the driver as is, but adding a comment
could be useful.

Thanks,

Gregory



> ---
>  drivers/ata/sata_mv.c | 13 ++++++++-----
>  1 file changed, 8 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/ata/sata_mv.c b/drivers/ata/sata_mv.c
> index b66bcda88320..3b2246dded74 100644
> --- a/drivers/ata/sata_mv.c
> +++ b/drivers/ata/sata_mv.c
> @@ -4067,7 +4067,6 @@ static int mv_platform_probe(struct platform_device *pdev)
>  	struct ata_host *host;
>  	struct mv_host_priv *hpriv;
>  	struct resource *res;
> -	void __iomem *mmio;
>  	int n_ports = 0, irq = 0;
>  	int rc;
>  	int port;
> @@ -4086,9 +4085,8 @@ static int mv_platform_probe(struct platform_device *pdev)
>  	 * Get the register base first
>  	 */
>  	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> -	mmio = devm_ioremap_resource(&pdev->dev, res);
> -	if (IS_ERR(mmio))
> -		return PTR_ERR(mmio);
> +	if (res == NULL)
> +		return -EINVAL;
>  
>  	/* allocate host */
>  	if (pdev->dev.of_node) {
> @@ -4132,7 +4130,12 @@ static int mv_platform_probe(struct platform_device *pdev)
>  	hpriv->board_idx = chip_soc;
>  
>  	host->iomap = NULL;
> -	hpriv->base = mmio - SATAHC0_REG_BASE;
> +	hpriv->base = devm_ioremap(&pdev->dev, res->start,
> +				   resource_size(res));
> +	if (!hpriv->base)
> +		return -ENOMEM;
> +
> +	hpriv->base -= SATAHC0_REG_BASE;
>  
>  	hpriv->clk = clk_get(&pdev->dev, NULL);
>  	if (IS_ERR(hpriv->clk))
> -- 
> 2.11.0
>

-- 
Gregory Clement, Free Electrons
Kernel, drivers, real-time and embedded Linux
development, consulting, training and support.
http://free-electrons.com

^ permalink raw reply

* Re: [PATCH V3 1/3] ata: ahci_tegra: Add AHCI support for tegra210
From: Mikko Perttunen @ 2017-05-24  7:15 UTC (permalink / raw)
  To: Preetham Chandru,
	thierry.reding-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org,
	tj-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org
  Cc: linux-ide-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	Laxman Dewangan,
	preetham260-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org,
	Venu Byravarasu, Pavan Kunapuli,
	linux-tegra-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
In-Reply-To: <6a39df12-c51d-d5b5-c126-1f264bb00ad0-/1wQRMveznE@public.gmane.org>

Also Vagrant Cascadian mentioned on IRC that the call to 
tegra_ahci_handle_quirks is currently before the definition of the 
function which causes warnings or compilation errors. So please add a 
declaration of the function before the call.

Cheers,
Mikko.

On 24.05.2017 10:14, Mikko Perttunen wrote:
> On 24.05.2017 08:44, Preetham Chandru wrote:
>> ...

^ permalink raw reply

* Re: [PATCH V3 1/3] ata: ahci_tegra: Add AHCI support for tegra210
From: Mikko Perttunen @ 2017-05-24  7:14 UTC (permalink / raw)
  To: Preetham Chandru,
	thierry.reding-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org,
	tj-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org
  Cc: linux-ide-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	Laxman Dewangan,
	preetham260-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org,
	Venu Byravarasu, Pavan Kunapuli,
	linux-tegra-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
In-Reply-To: <db35147839414c609a5dc2cde44dd998-gjLx+0+SZqK6sJks/06JalaTQe2KTcn/@public.gmane.org>

On 24.05.2017 08:44, Preetham Chandru wrote:
>
>
>> -----Original Message-----
>> From: Mikko Perttunen [mailto:cyndis-/1wQRMveznE@public.gmane.org]
>> Sent: Sunday, May 14, 2017 4:49 PM
>> To: Preetham Chandru <pchandru-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>; thierry.reding-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org;
>> tj-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org
>> Cc: tegra-u79uwXL29TY76Z2rM5mHXA@public.gmane.org; linux-ide-u79uwXL29TY76Z2rM5mHXA@public.gmane.org; Laxman Dewangan
>> <ldewangan-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>; preetham260-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org; Venu Byravarasu
>> <vbyravarasu-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>; Pavan Kunapuli <pkunapuli-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>; linux-
>> tegra-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
>> Subject: Re: [PATCH V3 1/3] ata: ahci_tegra: Add AHCI support for tegra210
>>
>> On 05/12/2017 12:55 PM, Preetham Chandru wrote:
>>> +CC linux-tegra-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
>>>
>>>> -----Original Message-----
>>>> From: Preetham Chandru
>>>> Sent: Friday, May 12, 2017 3:04 PM
>>>> To: thierry.reding-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org; tj-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org
>>>> Cc: tegra-u79uwXL29TY76Z2rM5mHXA@public.gmane.org; linux-ide-u79uwXL29TY76Z2rM5mHXA@public.gmane.org; Laxman Dewangan
>>>> <ldewangan-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>; preetham260-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org; Venu Byravarasu
>>>> <vbyravarasu-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>; Pavan Kunapuli <pkunapuli-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>;
>>>> Preetham Chandru <pchandru-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>
>>>> Subject: [PATCH V3 1/3] ata: ahci_tegra: Add AHCI support for
>>>> tegra210
>>>>
>>>> From: Preetham Chandru R <pchandru-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>
>>>>
>>>> 1. Move tegra124 specifics to tegra124_ahci_init.
>>>> 2. Separate the regulators needed for tegra124 and tegra210.
>>>> 3. Disable DIPM and Devslp for t210 and t124 as there are known
>>>> issues
>>>>
>>>> Signed-off-by: Preetham Chandru R <pchandru-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>
>>>> ---
>>>> v3:
>>>> * Remove inline functions for read/write and modify to  SATA, SATA
>>>> Config and SATA Aux registers.
>>>> * Add code to disable DIPM and DevSlp for t210 and t124
>>>> v2:
>>>> * Fix indentation issues
>>>> * Move the change to disable DIPM, HIPM, DevSlp, partial,  slumber
>>>> and NCQ into a separate patch
>>>> ---
>>>> drivers/ata/ahci_tegra.c | 377
>>>> ++++++++++++++++++++++++++++++++++++-------
>>>> ----
>>>> 1 file changed, 293 insertions(+), 84 deletions(-)
>>>>
>>>> diff --git a/drivers/ata/ahci_tegra.c b/drivers/ata/ahci_tegra.c
>>>> index
>>>> 3a62eb2..cd581a6 100644
>>>> --- a/drivers/ata/ahci_tegra.c
>>>> +++ b/drivers/ata/ahci_tegra.c
>>>> @@ -34,7 +34,8 @@
>>>> #define DRV_NAME "tegra-ahci"
>>>>
>>>> #define SATA_CONFIGURATION_0				0x180
>>>> -#define SATA_CONFIGURATION_EN_FPCI			BIT(0)
>>>> +#define SATA_CONFIGURATION_0_EN_FPCI			BIT(0)
>>>> +#define SATA_CONFIGURATION_0_CLK_OVERRIDE			BIT(31)
>>>>
>>>> #define SCFG_OFFSET					0x1000
>>>>
>>>> @@ -45,17 +46,55 @@
>>>> #define T_SATA0_CFG_1_SERR				BIT(8)
>>>>
>>>> #define T_SATA0_CFG_9					0x24
>>>> -#define T_SATA0_CFG_9_BASE_ADDRESS_SHIFT		13
>>>> +#define T_SATA0_CFG_9_BASE_ADDRESS			0x40020000
>>>>
>>>> #define SATA_FPCI_BAR5					0x94
>>>> -#define SATA_FPCI_BAR5_START_SHIFT			4
>>>> +#define SATA_FPCI_BAR5_START_MASK			(0xfffffff << 4)
>>>> +#define SATA_FPCI_BAR5_START				(0x0040020 <<
>>>> 4)
>>>> +#define SATA_FPCI_BAR5_ACCESS_TYPE			(0x1)
>>>>
>>>> #define SATA_INTR_MASK					0x188
>>>> #define SATA_INTR_MASK_IP_INT_MASK			BIT(16)
>>>>
>>>> +#define T_SATA0_CFG_35					0x94
>>>> +#define T_SATA0_CFG_35_IDP_INDEX_MASK			(0x7ff
>> << 2)
>>>> +#define T_SATA0_CFG_35_IDP_INDEX			(0x2a << 2)
>>>> +
>>>> +#define T_SATA0_AHCI_IDP1				0x98
>>>> +#define T_SATA0_AHCI_IDP1_DATA				(0x400040)
>>>> +
>>>> +#define T_SATA0_CFG_PHY_1				0x12c
>>>> +#define T_SATA0_CFG_PHY_1_PADS_IDDQ_EN			BIT(23)
>>>> +#define T_SATA0_CFG_PHY_1_PAD_PLL_IDDQ_EN		BIT(22)
>>>> +
>>>> +#define T_SATA0_NVOOB                                   0x114
>>>> +#define T_SATA0_NVOOB_COMMA_CNT_MASK                    (0xff << 16)
>>>> +#define T_SATA0_NVOOB_COMMA_CNT                         (0x07 << 16)
>>>> +#define T_SATA0_NVOOB_SQUELCH_FILTER_MODE_MASK          (0x3 << 24)
>>>> +#define T_SATA0_NVOOB_SQUELCH_FILTER_MODE               (0x1 << 24)
>>>> +#define T_SATA0_NVOOB_SQUELCH_FILTER_LENGTH_MASK        (0x3 << 26)
>>>> +#define T_SATA0_NVOOB_SQUELCH_FILTER_LENGTH             (0x3 << 26)
>>>> +
>>>> +#define T_SATA_CFG_PHY_0                                0x120
>>>> +#define T_SATA_CFG_PHY_0_USE_7BIT_ALIGN_DET_FOR_SPD     BIT(11)
>>>> +#define T_SATA_CFG_PHY_0_MASK_SQUELCH                   BIT(24)
>>>> +
>>>> +#define T_SATA0_CFG2NVOOB_2				0x134
>>>> +#define T_SATA0_CFG2NVOOB_2_COMWAKE_IDLE_CNT_LOW_MASK
>> 	(0x1ff
>>>> << 18)
>>>> +#define T_SATA0_CFG2NVOOB_2_COMWAKE_IDLE_CNT_LOW	(0xc <<
>> 18)
>>>> +
>>>> #define T_SATA0_AHCI_HBA_CAP_BKDR			0x300
>>>> +#define T_SATA0_AHCI_HBA_CAP_BKDR_PARTIAL_ST_CAP	BIT(13)
>>>> +#define T_SATA0_AHCI_HBA_CAP_BKDR_SLUMBER_ST_CAP	BIT(14)
>>>> +#define T_SATA0_AHCI_HBA_CAP_BKDR_SALP			BIT(26)
>>>> +#define T_SATA0_AHCI_HBA_CAP_BKDR_SUPP_PM		BIT(17)
>>>> +#define T_SATA0_AHCI_HBA_CAP_BKDR_SNCQ			BIT(30)
>>>>
>>>> #define T_SATA0_BKDOOR_CC				0x4a4
>>>> +#define T_SATA0_BKDOOR_CC_CLASS_CODE_MASK		(0xffff << 16)
>>>> +#define T_SATA0_BKDOOR_CC_CLASS_CODE
>> 	(0x0106 << 16)
>>>> +#define T_SATA0_BKDOOR_CC_PROG_IF_MASK			(0xff <<
>> 8)
>>>> +#define T_SATA0_BKDOOR_CC_PROG_IF			(0x01 << 8)
>>>>
>>>> #define T_SATA0_CFG_SATA				0x54c
>>>> #define T_SATA0_CFG_SATA_BACKDOOR_PROG_IF_EN		BIT(12)
>>>> @@ -82,9 +121,35 @@
>>>> #define T_SATA0_CHX_PHY_CTRL11				0x6d0
>>>> #define T_SATA0_CHX_PHY_CTRL11_GEN2_RX_EQ		(0x2800 << 16)
>>>>
>>>> +#define T_SATA0_CHX_PHY_CTRL17_0			0x6e8
>>>> +#define T_SATA0_CHX_PHY_CTRL17_0_RX_EQ_CTRL_L_GEN1
>> 	0x55010000
>>>> +#define T_SATA0_CHX_PHY_CTRL18_0			0x6ec
>>>> +#define T_SATA0_CHX_PHY_CTRL18_0_RX_EQ_CTRL_L_GEN2
>> 	0x55010000
>>>> +#define T_SATA0_CHX_PHY_CTRL20_0			0x6f4
>>>> +#define T_SATA0_CHX_PHY_CTRL20_0_RX_EQ_CTRL_H_GEN1	0x1
>>>> +#define T_SATA0_CHX_PHY_CTRL21_0			0x6f8
>>>> +#define T_SATA0_CHX_PHY_CTRL21_0_RX_EQ_CTRL_H_GEN2	0x1
>>>> +
>>>> +/* AUX Registers */
>>>> +#define SATA_AUX_MISC_CNTL_1_0				0x8
>>>> +#define SATA_AUX_MISC_CNTL_1_0_DEVSLP_OVERRIDE		BIT(17)
>>>> +#define SATA_AUX_MISC_CNTL_1_0_SDS_SUPPORT		BIT(13)
>>>> +#define SATA_AUX_MISC_CNTL_1_0_DESO_SUPPORT		BIT(15)
>>>> +
>>>> +#define SATA_AUX_RX_STAT_INT_0				0xc
>>>> +#define SATA_AUX_RX_STAT_INT_0_SATA_DEVSLP		BIT(7)
>>>> +
>>>> +#define SATA_AUX_SPARE_CFG0_0				0x18
>>>> +#define SATA_AUX_SPARE_CFG0_0_MDAT_TIMER_AFTER_PG_VALID	BIT(14)
>>>> +
>>>> #define FUSE_SATA_CALIB					0x124
>>>> #define FUSE_SATA_CALIB_MASK				0x3
>>>>
>>>> +enum {
>>>> +	NO_DEVSLP	= (1 << 0),
>>>> +	NO_DIPM		= (1 << 1),
>>>> +};
>>>> +
>>>> struct sata_pad_calibration {
>>>> 	u8 gen1_tx_amp;
>>>> 	u8 gen1_tx_peak;
>>>> @@ -99,15 +164,89 @@ static const struct sata_pad_calibration
>>>> tegra124_pad_calibration[] = {
>>>> 	{0x14, 0x0e, 0x1a, 0x0e},
>>>> };
>>>>
>>>> +struct tegra_ahci_ops {
>>>> +	int (*init)(struct ahci_host_priv *); };
>>>> +
>>>> +struct tegra_ahci_soc {
>>>> +	const char *const	*supply_names;
>>>> +	u32			num_supplies;
>>>> +	u32			quirks;
>>>> +	struct tegra_ahci_ops	ops;
>>>> +};
>>>> +
>>>> struct tegra_ahci_priv {
>>>> 	struct platform_device	   *pdev;
>>>> 	void __iomem		   *sata_regs;
>>>> +	void __iomem		   *sata_aux_regs;
>>>> 	struct reset_control	   *sata_rst;
>>>> 	struct reset_control	   *sata_oob_rst;
>>>> 	struct reset_control	   *sata_cold_rst;
>>>> 	/* Needs special handling, cannot use ahci_platform */
>>>> 	struct clk		   *sata_clk;
>>>> -	struct regulator_bulk_data supplies[5];
>>>> +	struct regulator_bulk_data *supplies;
>>>> +	struct tegra_ahci_soc	   *soc_data;
>>>> +};
>>>> +
>>>> +static const char *const tegra124_supply_names[] = {
>>>> +	"avdd", "hvdd", "vddio", "target-5v", "target-12v"
>>>> +};
>>>> +
>>>> +static int tegra124_ahci_init(struct ahci_host_priv *hpriv) {
>>>> +	struct tegra_ahci_priv *tegra = hpriv->plat_data;
>>>> +	struct sata_pad_calibration calib;
>>>> +	int ret;
>>>> +	u32 val;
>>>> +
>>>> +	/* Pad calibration */
>>>> +	ret = tegra_fuse_readl(FUSE_SATA_CALIB, &val);
>>>> +	if (ret)
>>>> +		return ret;
>>>> +
>>>> +	calib = tegra124_pad_calibration[val & FUSE_SATA_CALIB_MASK];
>>>> +
>>>> +	writel(BIT(0), tegra->sata_regs + SCFG_OFFSET + T_SATA0_INDEX);
>>>> +
>>>> +	val = readl(tegra->sata_regs +
>>>> +		    SCFG_OFFSET + T_SATA0_CHX_PHY_CTRL1_GEN1);
>>>> +	val &= ~T_SATA0_CHX_PHY_CTRL1_GEN1_TX_AMP_MASK;
>>>> +	val &= ~T_SATA0_CHX_PHY_CTRL1_GEN1_TX_PEAK_MASK;
>>>> +	val |= calib.gen1_tx_amp <<
>>>> T_SATA0_CHX_PHY_CTRL1_GEN1_TX_AMP_SHIFT;
>>>> +	val |= calib.gen1_tx_peak <<
>>>> T_SATA0_CHX_PHY_CTRL1_GEN1_TX_PEAK_SHIFT;
>>>> +	writel(val, tegra->sata_regs + SCFG_OFFSET +
>>>> +	       T_SATA0_CHX_PHY_CTRL1_GEN1);
>>>> +
>>>> +	val = readl(tegra->sata_regs +
>>>> +		    SCFG_OFFSET + T_SATA0_CHX_PHY_CTRL1_GEN2);
>>>> +	val &= ~T_SATA0_CHX_PHY_CTRL1_GEN2_TX_AMP_MASK;
>>>> +	val &= ~T_SATA0_CHX_PHY_CTRL1_GEN2_TX_PEAK_MASK;
>>>> +	val |= calib.gen2_tx_amp <<
>>>> T_SATA0_CHX_PHY_CTRL1_GEN1_TX_AMP_SHIFT;
>>>> +	val |= calib.gen2_tx_peak <<
>>>> T_SATA0_CHX_PHY_CTRL1_GEN1_TX_PEAK_SHIFT;
>>>> +	writel(val, tegra->sata_regs + SCFG_OFFSET +
>>>> +	       T_SATA0_CHX_PHY_CTRL1_GEN2);
>>>> +
>>>> +	writel(T_SATA0_CHX_PHY_CTRL11_GEN2_RX_EQ,
>>>> +	       tegra->sata_regs + SCFG_OFFSET + T_SATA0_CHX_PHY_CTRL11);
>>>> +	writel(T_SATA0_CHX_PHY_CTRL2_CDR_CNTL_GEN1,
>>>> +	       tegra->sata_regs + SCFG_OFFSET + T_SATA0_CHX_PHY_CTRL2);
>>>> +
>>>> +	writel(0, tegra->sata_regs + SCFG_OFFSET + T_SATA0_INDEX);
>>>> +
>>>> +	return 0;
>>>> +}
>>>> +
>>>> +static const struct tegra_ahci_soc tegra124_ahci_soc_data = {
>>>> +	.supply_names = tegra124_supply_names,
>>>> +	.num_supplies = ARRAY_SIZE(tegra124_supply_names),
>>>> +	.quirks = NO_DIPM | NO_DEVSLP,
>>>> +	.ops = {
>>>> +		.init = tegra124_ahci_init,
>>>> +		},
>>
>> Indent this "}," one tab less (i.e. to same level as '.ops')
>>
> Okay.
>
>>>> +};
>>>> +
>>>> +static const struct tegra_ahci_soc tegra210_ahci_soc_data = {
>>>> +	.quirks = NO_DIPM | NO_DEVSLP,
>>>> };
>>>>
>>>> static int tegra_ahci_power_on(struct ahci_host_priv *hpriv) @@
>>>> -115,7 +254,7 @@ static int tegra_ahci_power_on(struct ahci_host_priv
>> *hpriv)
>>>> 	struct tegra_ahci_priv *tegra = hpriv->plat_data;
>>>> 	int ret;
>>>>
>>>> -	ret = regulator_bulk_enable(ARRAY_SIZE(tegra->supplies),
>>>> +	ret = regulator_bulk_enable(tegra->soc_data->num_supplies,
>>>> 				    tegra->supplies);
>>>> 	if (ret)
>>>> 		return ret;
>>>> @@ -144,8 +283,7 @@ static int tegra_ahci_power_on(struct
>>>> ahci_host_priv
>>>> *hpriv)
>>>> 	tegra_powergate_power_off(TEGRA_POWERGATE_SATA);
>>>>
>>>> disable_regulators:
>>>> -	regulator_bulk_disable(ARRAY_SIZE(tegra->supplies), tegra->supplies);
>>>> -
>>>> +	regulator_bulk_disable(tegra->soc_data->num_supplies,
>>>> +tegra->supplies);
>>
>> Indent this.
>>
> This seems to be issue with my outlook. I don't see this issue
> when I sent first using git send-email. Will take care of this.
>
>>>> 	return ret;
>>>> }
>>>>
>>>> @@ -162,97 +300,137 @@ static void tegra_ahci_power_off(struct
>>>> ahci_host_priv *hpriv)
>>>> 	clk_disable_unprepare(tegra->sata_clk);
>>>> 	tegra_powergate_power_off(TEGRA_POWERGATE_SATA);
>>>>
>>>> -	regulator_bulk_disable(ARRAY_SIZE(tegra->supplies), tegra->supplies);
>>>> +	regulator_bulk_disable(tegra->soc_data->num_supplies,
>>>> +tegra->supplies);
>>
>> And this
>>
> This again seems to be issue with my outlook. Will correct it.
>>>> }
>>>>
>>>> static int tegra_ahci_controller_init(struct ahci_host_priv *hpriv)  {
>>>> 	struct tegra_ahci_priv *tegra = hpriv->plat_data;
>>>> 	int ret;
>>>> -	unsigned int val;
>>>> -	struct sata_pad_calibration calib;
>>>> +	u32 val;
>>>>
>>>> 	ret = tegra_ahci_power_on(hpriv);
>>>> -	if (ret) {
>>>> -		dev_err(&tegra->pdev->dev,
>>>> -			"failed to power on AHCI controller: %d\n", ret);
>>>> +	if (ret)
>>>> 		return ret;
>>>> -	}
>>>>
>>>> +	/*
>>>> +	 * Program the following  SATA IPFS registers
>>
>> Extra space between "following" and "SATA"
>>
> Okay, will correct it.
>>>> +	 * to allow SW accesses to SATA's MMIO Register
>>
>> Perhaps say "register range"
>>
>>>> +	 */
>>>> +	val = readl(tegra->sata_regs + SATA_FPCI_BAR5);
>>>> +	val &= ~(SATA_FPCI_BAR5_START_MASK |
>>>> SATA_FPCI_BAR5_ACCESS_TYPE);
>>>> +	val |= SATA_FPCI_BAR5_START | SATA_FPCI_BAR5_ACCESS_TYPE;
>>>> +	writel(val, tegra->sata_regs + SATA_FPCI_BAR5);
>>>> +
>>>> +	/* Program the following SATA IPFS register to enable the SATA */
>>>> 	val = readl(tegra->sata_regs + SATA_CONFIGURATION_0);
>>>> -	val |= SATA_CONFIGURATION_EN_FPCI;
>>>> +	val |= SATA_CONFIGURATION_0_EN_FPCI;
>>>> 	writel(val, tegra->sata_regs + SATA_CONFIGURATION_0);
>>>>
>>>> -	/* Pad calibration */
>>>> -
>>>> -	ret = tegra_fuse_readl(FUSE_SATA_CALIB, &val);
>>>> -	if (ret) {
>>>> -		dev_err(&tegra->pdev->dev,
>>>> -			"failed to read calibration fuse: %d\n", ret);
>>>> -		return ret;
>>>> -	}
>>>> -
>>>> -	calib = tegra124_pad_calibration[val & FUSE_SATA_CALIB_MASK];
>>>> -
>>>> -	writel(BIT(0), tegra->sata_regs + SCFG_OFFSET + T_SATA0_INDEX);
>>>> -
>>>> -	val = readl(tegra->sata_regs +
>>>> -		SCFG_OFFSET + T_SATA0_CHX_PHY_CTRL1_GEN1);
>>>> -	val &= ~T_SATA0_CHX_PHY_CTRL1_GEN1_TX_AMP_MASK;
>>>> -	val &= ~T_SATA0_CHX_PHY_CTRL1_GEN1_TX_PEAK_MASK;
>>>> -	val |= calib.gen1_tx_amp <<
>>>> -			T_SATA0_CHX_PHY_CTRL1_GEN1_TX_AMP_SHIFT;
>>>> -	val |= calib.gen1_tx_peak <<
>>>> -			T_SATA0_CHX_PHY_CTRL1_GEN1_TX_PEAK_SHIFT;
>>>> -	writel(val, tegra->sata_regs + SCFG_OFFSET +
>>>> -		T_SATA0_CHX_PHY_CTRL1_GEN1);
>>>> -
>>>> -	val = readl(tegra->sata_regs +
>>>> -			SCFG_OFFSET + T_SATA0_CHX_PHY_CTRL1_GEN2);
>>>> -	val &= ~T_SATA0_CHX_PHY_CTRL1_GEN2_TX_AMP_MASK;
>>>> -	val &= ~T_SATA0_CHX_PHY_CTRL1_GEN2_TX_PEAK_MASK;
>>>> -	val |= calib.gen2_tx_amp <<
>>>> -			T_SATA0_CHX_PHY_CTRL1_GEN1_TX_AMP_SHIFT;
>>>> -	val |= calib.gen2_tx_peak <<
>>>> -			T_SATA0_CHX_PHY_CTRL1_GEN1_TX_PEAK_SHIFT;
>>>> -	writel(val, tegra->sata_regs + SCFG_OFFSET +
>>>> -		T_SATA0_CHX_PHY_CTRL1_GEN2);
>>>> -
>>>> -	writel(T_SATA0_CHX_PHY_CTRL11_GEN2_RX_EQ,
>>>> -		tegra->sata_regs + SCFG_OFFSET + T_SATA0_CHX_PHY_CTRL11);
>>>> -	writel(T_SATA0_CHX_PHY_CTRL2_CDR_CNTL_GEN1,
>>>> -		tegra->sata_regs + SCFG_OFFSET + T_SATA0_CHX_PHY_CTRL2);
>>>> -
>>>> -	writel(0, tegra->sata_regs + SCFG_OFFSET + T_SATA0_INDEX);
>>>> -
>>>> -	/* Program controller device ID */
>>>> +	/* Electrical settings for better link stability */
>>>> +	val = T_SATA0_CHX_PHY_CTRL17_0_RX_EQ_CTRL_L_GEN1;
>>>> +	writel(val, tegra->sata_regs + SCFG_OFFSET +
>>>> T_SATA0_CHX_PHY_CTRL17_0);
>>>> +	val = T_SATA0_CHX_PHY_CTRL18_0_RX_EQ_CTRL_L_GEN2;
>>>> +	writel(val, tegra->sata_regs + SCFG_OFFSET +
>>>> T_SATA0_CHX_PHY_CTRL18_0);
>>>> +	val = T_SATA0_CHX_PHY_CTRL20_0_RX_EQ_CTRL_H_GEN1;
>>>> +	writel(val, tegra->sata_regs + SCFG_OFFSET +
>>>> T_SATA0_CHX_PHY_CTRL20_0);
>>>> +	val = T_SATA0_CHX_PHY_CTRL21_0_RX_EQ_CTRL_H_GEN2;
>>>> +	writel(val, tegra->sata_regs + SCFG_OFFSET +
>>>> +T_SATA0_CHX_PHY_CTRL21_0);
>>
>> Indent
>>
>>>> +
>>>> +	/* For SQUELCH Filter & Gen3 drive getting detected as Gen1 drive
>>>> +*/
>>>> +
>>>> +	val = readl(tegra->sata_regs + SCFG_OFFSET + T_SATA_CFG_PHY_0);
>>>> +	val |= T_SATA_CFG_PHY_0_MASK_SQUELCH;
>>>> +	val &= ~T_SATA_CFG_PHY_0_USE_7BIT_ALIGN_DET_FOR_SPD;
>>>> +	writel(val, tegra->sata_regs + SCFG_OFFSET + T_SATA_CFG_PHY_0);
>>>> +
>>>> +	val = readl(tegra->sata_regs + SCFG_OFFSET + T_SATA0_NVOOB);
>>>> +	val &= ~(T_SATA0_NVOOB_COMMA_CNT_MASK |
>>>> +		 T_SATA0_NVOOB_SQUELCH_FILTER_LENGTH_MASK |
>>>> +		 T_SATA0_NVOOB_SQUELCH_FILTER_MODE_MASK);
>>>> +	val |= (T_SATA0_NVOOB_COMMA_CNT |
>>>> +		T_SATA0_NVOOB_SQUELCH_FILTER_LENGTH |
>>>> +		T_SATA0_NVOOB_SQUELCH_FILTER_MODE);
>>>> +	writel(val, tegra->sata_regs + SCFG_OFFSET + T_SATA0_NVOOB);
>>>> +
>>>> +	/*
>>>> +	 * Change CFG2NVOOB_2_COMWAKE_IDLE_CNT_LOW from 83.3 ns to
>>>> 58.8ns
>>>> +	 */
>>>> +	val = readl(tegra->sata_regs + SCFG_OFFSET + T_SATA0_CFG2NVOOB_2);
>>>> +	val &= ~T_SATA0_CFG2NVOOB_2_COMWAKE_IDLE_CNT_LOW_MASK;
>>>> +	val |= T_SATA0_CFG2NVOOB_2_COMWAKE_IDLE_CNT_LOW;
>>>> +	writel(val, tegra->sata_regs + SCFG_OFFSET + T_SATA0_CFG2NVOOB_2);
>>>> +
>>>> +	if (tegra->soc_data->ops.init)
>>>> +		tegra->soc_data->ops.init(hpriv);
>>>> +
>>>> +	/*
>>>> +	 * Program the following SATA configuration registers
>>>> +	 * to initialize SATA
>>>> +	 */
>>>> +	val = readl(tegra->sata_regs + SCFG_OFFSET + T_SATA0_CFG_1);
>>>> +	val |= (T_SATA0_CFG_1_IO_SPACE | T_SATA0_CFG_1_MEMORY_SPACE |
>>>> +		T_SATA0_CFG_1_BUS_MASTER | T_SATA0_CFG_1_SERR);
>>>> +	writel(val, tegra->sata_regs + SCFG_OFFSET + T_SATA0_CFG_1);
>>>> +	val = T_SATA0_CFG_9_BASE_ADDRESS;
>>>> +	writel(val, tegra->sata_regs + SCFG_OFFSET + T_SATA0_CFG_9);
>>>>
>>>> +	/* Program Class Code and Programming interface for SATA */
>>>> 	val = readl(tegra->sata_regs + SCFG_OFFSET + T_SATA0_CFG_SATA);
>>>> 	val |= T_SATA0_CFG_SATA_BACKDOOR_PROG_IF_EN;
>>>> 	writel(val, tegra->sata_regs + SCFG_OFFSET + T_SATA0_CFG_SATA);
>>>>
>>>> -	writel(0x01060100, tegra->sata_regs + SCFG_OFFSET +
>>>> T_SATA0_BKDOOR_CC);
>>>> +	val = readl(tegra->sata_regs + SCFG_OFFSET + T_SATA0_BKDOOR_CC);
>>>> +	val &=
>>>> +	    ~(T_SATA0_BKDOOR_CC_CLASS_CODE_MASK |
>>>> +	      T_SATA0_BKDOOR_CC_PROG_IF_MASK);
>>>> +	val |= T_SATA0_BKDOOR_CC_CLASS_CODE |
>>>> T_SATA0_BKDOOR_CC_PROG_IF;
>>>> +	writel(val, tegra->sata_regs + SCFG_OFFSET + T_SATA0_BKDOOR_CC);
>>>>
>>>> 	val = readl(tegra->sata_regs + SCFG_OFFSET + T_SATA0_CFG_SATA);
>>>> 	val &= ~T_SATA0_CFG_SATA_BACKDOOR_PROG_IF_EN;
>>>> 	writel(val, tegra->sata_regs + SCFG_OFFSET + T_SATA0_CFG_SATA);
>>>>
>>>> -	/* Enable IO & memory access, bus master mode */
>>>> -
>>>> -	val = readl(tegra->sata_regs + SCFG_OFFSET + T_SATA0_CFG_1);
>>>> -	val |= T_SATA0_CFG_1_IO_SPACE | T_SATA0_CFG_1_MEMORY_SPACE |
>>>> -		T_SATA0_CFG_1_BUS_MASTER | T_SATA0_CFG_1_SERR;
>>>> -	writel(val, tegra->sata_regs + SCFG_OFFSET + T_SATA0_CFG_1);
>>>> -
>>>> -	/* Program SATA MMIO */
>>>> -
>>>> -	writel(0x10000 << SATA_FPCI_BAR5_START_SHIFT,
>>>> -	       tegra->sata_regs + SATA_FPCI_BAR5);
>>>> -
>>>> -	writel(0x08000 << T_SATA0_CFG_9_BASE_ADDRESS_SHIFT,
>>>> -	       tegra->sata_regs + SCFG_OFFSET + T_SATA0_CFG_9);
>>>> +	/* Enabling LPM capabilities through Backdoor Programming */
>>>> +	val = readl(tegra->sata_regs + SCFG_OFFSET +
>>>> T_SATA0_AHCI_HBA_CAP_BKDR);
>>>> +	val |= (T_SATA0_AHCI_HBA_CAP_BKDR_PARTIAL_ST_CAP |
>>>> +		T_SATA0_AHCI_HBA_CAP_BKDR_SLUMBER_ST_CAP |
>>>> +		T_SATA0_AHCI_HBA_CAP_BKDR_SALP |
>>>> +		T_SATA0_AHCI_HBA_CAP_BKDR_SUPP_PM);
>>>> +	writel(val, tegra->sata_regs + SCFG_OFFSET +
>>>> +T_SATA0_AHCI_HBA_CAP_BKDR);
>>>> +
>>>> +	/* SATA Second Level Clock Gating configuration
>>>> +	 * Enabling Gating of Tx/Rx clocks and driving Pad IDDQ and Lane
>>>> +	 * IDDQ Signals
>>>> +	 */
>>>> +	val = readl(tegra->sata_regs + SCFG_OFFSET + T_SATA0_CFG_35);
>>>> +	val &= ~T_SATA0_CFG_35_IDP_INDEX_MASK;
>>>> +	val |= T_SATA0_CFG_35_IDP_INDEX;
>>>> +	writel(val, tegra->sata_regs + SCFG_OFFSET + T_SATA0_CFG_35);
>>>> +
>>>> +	val = T_SATA0_AHCI_IDP1_DATA;
>>>> +	writel(val, tegra->sata_regs + SCFG_OFFSET + T_SATA0_AHCI_IDP1);
>>>> +
>>>> +	val = readl(tegra->sata_regs + SCFG_OFFSET + T_SATA0_CFG_PHY_1);
>>>> +	val |= (T_SATA0_CFG_PHY_1_PADS_IDDQ_EN |
>>>> +		T_SATA0_CFG_PHY_1_PAD_PLL_IDDQ_EN);
>>>> +	writel(val, tegra->sata_regs + SCFG_OFFSET + T_SATA0_CFG_PHY_1);
>>>> +
>>>> +	/*
>>>> +	 *  Indicate Sata only has the capability to enter DevSleep
>>
>> Stray space in front
>>
> Okay, will correct it.
>>>> +	 * from slumber link.
>>>> +	 */
>>>> +
>>>> +	val = readl(tegra->sata_aux_regs + SATA_AUX_MISC_CNTL_1_0);
>>>> +	val |= SATA_AUX_MISC_CNTL_1_0_DESO_SUPPORT;
>>>> +	writel(val, tegra->sata_aux_regs + SATA_AUX_MISC_CNTL_1_0);
>>>> +
>>>> +	/* Enabling IPFS Clock Gating */
>>>> +	val = readl(tegra->sata_regs + SATA_CONFIGURATION_0);
>>>> +	val &= ~SATA_CONFIGURATION_0_CLK_OVERRIDE;
>>>> +	writel(val, tegra->sata_regs + SATA_CONFIGURATION_0);
>>>>
>>>> -	/* Unmask SATA interrupts */
>>>> +	tegra_ahci_handle_quirks(hpriv);
>>>>
>>>> 	val = readl(tegra->sata_regs + SATA_INTR_MASK);
>>>> 	val |= SATA_INTR_MASK_IP_INT_MASK;
>>>> @@ -278,7 +456,7 @@ static struct ata_port_operations
>>>> ahci_tegra_port_ops = {
>>>> 	.host_stop	= tegra_ahci_host_stop,
>>>> };
>>>>
>>>> -static const struct ata_port_info ahci_tegra_port_info = {
>>>> +static struct ata_port_info ahci_tegra_port_info = {
>>>> 	.flags		= AHCI_FLAG_COMMON,
>>>> 	.pio_mask	= ATA_PIO4,
>>>> 	.udma_mask	= ATA_UDMA6,
>>>> @@ -286,7 +464,14 @@ static const struct ata_port_info
>>>> ahci_tegra_port_info = {  };
>>>>
>>>> static const struct of_device_id tegra_ahci_of_match[] = {
>>>> -	{ .compatible = "nvidia,tegra124-ahci" },
>>>> +	{
>>>> +		.compatible = "nvidia,tegra124-ahci",
>>>> +		.data = &tegra124_ahci_soc_data
>>>> +	},
>>>> +	{
>>>> +		.compatible = "nvidia,tegra210-ahci",
>>>> +		.data = &tegra210_ahci_soc_data
>>>> +	},
>>>> 	{}
>>>> };
>>>> MODULE_DEVICE_TABLE(of, tegra_ahci_of_match); @@ -295,12 +480,27
>> @@
>>>> static struct scsi_host_template ahci_platform_sht = {
>>>> 	AHCI_SHT(DRV_NAME),
>>>> };
>>>>
>>>> +static void tegra_ahci_handle_quirks(struct ahci_host_priv *hpriv) {
>>>> +	struct tegra_ahci_priv *tegra = hpriv->plat_data;
>>>> +	u32 val;
>>>> +
>>>> +	if (tegra->soc_data->quirks & NO_DEVSLP) {
>>>> +		val = readl(tegra->sata_aux_regs + SATA_AUX_MISC_CNTL_1_0);
>>>> +		val &= ~SATA_AUX_MISC_CNTL_1_0_SDS_SUPPORT;
>>>> +		writel(val, tegra->sata_aux_regs + SATA_AUX_MISC_CNTL_1_0);
>>>> +	} else if (tegra->soc_data->quirks & NO_DIPM) {
>>
>> I don't think this should be 'else if', but just two separate regular 'if's?
>>
> Okay, will correct it.
>>>> +		ahci_tegra_port_info.flags |= ATA_FLAG_NO_DIPM;
>>
>> Looking at the downstream device trees, it looks like DIPM and DEVSLP are still
>> disabled for Tegra186 - so why don't we just hardcode these quirks, by always
>> writing the MISC_CNTL_1_0 register and just adding ATA_FLAG_NO_DIPM to
>> ahci_tegra_port_info's static definition.
>>
>
> For Tegra186 the devslp pin is shared with PCIE clk req pin.
> In downstream PCIe driver by default assumes that CLKREQ is present and it owns it.
> So by default devslp is disabled. We verify devslp by making sure that pcie does not
> own this pin. This issue will be fixed in future chips.

Ok. How about DIPM, can we hardcode that? If not, or if it is fixed in 
Xavier, I think it would be cleaner to have two static port_info structs 
and select between then based on the quirk, so that we don't need to 
mutate the static data.

>
>>>> +	}
>>>> +}
>>>> +
>>>> static int tegra_ahci_probe(struct platform_device *pdev)  {
>>>> 	struct ahci_host_priv *hpriv;
>>>> 	struct tegra_ahci_priv *tegra;
>>>> 	struct resource *res;
>>>> 	int ret;
>>>> +	unsigned int i;
>>>>
>>>> 	hpriv = ahci_platform_get_resources(pdev);
>>>> 	if (IS_ERR(hpriv))
>>>> @@ -311,13 +511,18 @@ static int tegra_ahci_probe(struct
>>>> platform_device
>>>> *pdev)
>>>> 		return -ENOMEM;
>>>>
>>>> 	hpriv->plat_data = tegra;
>>>> -
>>>> 	tegra->pdev = pdev;
>>>> +	tegra->soc_data =
>>>> +	    (struct tegra_ahci_soc *)of_device_get_match_data(&pdev->dev);
>>>>
>>>> 	res = platform_get_resource(pdev, IORESOURCE_MEM, 1);
>>>> 	tegra->sata_regs = devm_ioremap_resource(&pdev->dev, res);
>>>> 	if (IS_ERR(tegra->sata_regs))
>>>> 		return PTR_ERR(tegra->sata_regs);
>>>> +	res = platform_get_resource(pdev, IORESOURCE_MEM, 2);
>>>> +	tegra->sata_aux_regs = devm_ioremap_resource(&pdev->dev, res);
>>>> +	if (IS_ERR(tegra->sata_aux_regs))
>>>> +		return PTR_ERR(tegra->sata_aux_regs);
>>
>> Requiring aux_regs would break backwards compatibility for Tegra124 device
>> trees. We will have to keep aux_regs optional for Tegra124, and skip writes to
>> them if they are not available.
>>
>
> As explained in the other patch, In current driver changes we are touching the
> devslp register to disable DevSlp which is a part of AUX for both t124 and t210.
> By default the devslp is enabled for T124 and T210. (Reset value is 1).
> I will add this aux register in T124 node as well.

According to device tree stability, the kernel must work with older 
device trees that had support for the device. So for T124, we need to 
continue to support older device trees without the aux_regs property.

>
>>>>
>>>> 	tegra->sata_rst = devm_reset_control_get(&pdev->dev, "sata");
>>>> 	if (IS_ERR(tegra->sata_rst)) {
>>>> @@ -343,13 +548,17 @@ static int tegra_ahci_probe(struct
>>>> platform_device
>>>> *pdev)
>>>> 		return PTR_ERR(tegra->sata_clk);
>>>> 	}
>>>>
>>>> -	tegra->supplies[0].supply = "avdd";
>>>> -	tegra->supplies[1].supply = "hvdd";
>>>> -	tegra->supplies[2].supply = "vddio";
>>>> -	tegra->supplies[3].supply = "target-5v";
>>>> -	tegra->supplies[4].supply = "target-12v";
>>>> +	tegra->supplies = devm_kcalloc(&pdev->dev,
>>>> +				       tegra->soc_data->num_supplies,
>>>> +				       sizeof(*tegra->supplies), GFP_KERNEL);
>>>> +	if (!tegra->supplies)
>>>> +		return -ENOMEM;
>>>> +
>>>> +	for (i = 0; i < tegra->soc_data->num_supplies; i++)
>>>> +		tegra->supplies[i].supply = tegra->soc_data->supply_names[i];
>>>>
>>>> -	ret = devm_regulator_bulk_get(&pdev->dev, ARRAY_SIZE(tegra-
>>>>> supplies),
>>>> +	ret = devm_regulator_bulk_get(&pdev->dev,
>>>> +				      tegra->soc_data->num_supplies,
>>>> 				      tegra->supplies);
>>>> 	if (ret) {
>>>> 		dev_err(&pdev->dev, "Failed to get regulators\n"); @@ -385,5
>>>> +594,5 @@ static struct platform_driver tegra_ahci_driver = {
>>>> module_platform_driver(tegra_ahci_driver);
>>>>
>>>> MODULE_AUTHOR("Mikko Perttunen <mperttunen-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>");
>>>> -MODULE_DESCRIPTION("Tegra124 AHCI SATA driver");
>>>> +MODULE_DESCRIPTION("Tegra AHCI SATA driver");
>>>> MODULE_LICENSE("GPL v2");
>>>> --
>>>> 2.1.4
>>>
>>> --
>>> To unsubscribe from this list: send the line "unsubscribe linux-tegra"
>>> in the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org More majordomo
>>> info at  http://vger.kernel.org/majordomo-info.html
>>>
>>
>> Thanks for posting!
>>
>> Mikko
> --
> To unsubscribe from this list: send the line "unsubscribe linux-tegra" in
> the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>

^ permalink raw reply

* RE: [PATCH V3 1/3] ata: ahci_tegra: Add AHCI support for tegra210
From: Preetham Chandru @ 2017-05-24  5:44 UTC (permalink / raw)
  To: Mikko Perttunen,
	thierry.reding-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org,
	tj-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org
  Cc: linux-ide-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	Laxman Dewangan,
	preetham260-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org,
	Venu Byravarasu, Pavan Kunapuli,
	linux-tegra-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
In-Reply-To: <3dfdc451-794a-832e-4985-ac56d7e1843e-/1wQRMveznE@public.gmane.org>



>-----Original Message-----
>From: Mikko Perttunen [mailto:cyndis-/1wQRMveznE@public.gmane.org]
>Sent: Sunday, May 14, 2017 4:49 PM
>To: Preetham Chandru <pchandru-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>; thierry.reding-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org;
>tj-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org
>Cc: tegra-u79uwXL29TY76Z2rM5mHXA@public.gmane.org; linux-ide-u79uwXL29TY76Z2rM5mHXA@public.gmane.org; Laxman Dewangan
><ldewangan-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>; preetham260-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org; Venu Byravarasu
><vbyravarasu-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>; Pavan Kunapuli <pkunapuli-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>; linux-
>tegra-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
>Subject: Re: [PATCH V3 1/3] ata: ahci_tegra: Add AHCI support for tegra210
>
>On 05/12/2017 12:55 PM, Preetham Chandru wrote:
>> +CC linux-tegra-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
>>
>>> -----Original Message-----
>>> From: Preetham Chandru
>>> Sent: Friday, May 12, 2017 3:04 PM
>>> To: thierry.reding-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org; tj-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org
>>> Cc: tegra-u79uwXL29TY76Z2rM5mHXA@public.gmane.org; linux-ide-u79uwXL29TY76Z2rM5mHXA@public.gmane.org; Laxman Dewangan
>>> <ldewangan-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>; preetham260-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org; Venu Byravarasu
>>> <vbyravarasu-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>; Pavan Kunapuli <pkunapuli-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>;
>>> Preetham Chandru <pchandru-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>
>>> Subject: [PATCH V3 1/3] ata: ahci_tegra: Add AHCI support for
>>> tegra210
>>>
>>> From: Preetham Chandru R <pchandru-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>
>>>
>>> 1. Move tegra124 specifics to tegra124_ahci_init.
>>> 2. Separate the regulators needed for tegra124 and tegra210.
>>> 3. Disable DIPM and Devslp for t210 and t124 as there are known
>>> issues
>>>
>>> Signed-off-by: Preetham Chandru R <pchandru-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>
>>> ---
>>> v3:
>>> * Remove inline functions for read/write and modify to  SATA, SATA
>>> Config and SATA Aux registers.
>>> * Add code to disable DIPM and DevSlp for t210 and t124
>>> v2:
>>> * Fix indentation issues
>>> * Move the change to disable DIPM, HIPM, DevSlp, partial,  slumber
>>> and NCQ into a separate patch
>>> ---
>>> drivers/ata/ahci_tegra.c | 377
>>> ++++++++++++++++++++++++++++++++++++-------
>>> ----
>>> 1 file changed, 293 insertions(+), 84 deletions(-)
>>>
>>> diff --git a/drivers/ata/ahci_tegra.c b/drivers/ata/ahci_tegra.c
>>> index
>>> 3a62eb2..cd581a6 100644
>>> --- a/drivers/ata/ahci_tegra.c
>>> +++ b/drivers/ata/ahci_tegra.c
>>> @@ -34,7 +34,8 @@
>>> #define DRV_NAME "tegra-ahci"
>>>
>>> #define SATA_CONFIGURATION_0				0x180
>>> -#define SATA_CONFIGURATION_EN_FPCI			BIT(0)
>>> +#define SATA_CONFIGURATION_0_EN_FPCI			BIT(0)
>>> +#define SATA_CONFIGURATION_0_CLK_OVERRIDE			BIT(31)
>>>
>>> #define SCFG_OFFSET					0x1000
>>>
>>> @@ -45,17 +46,55 @@
>>> #define T_SATA0_CFG_1_SERR				BIT(8)
>>>
>>> #define T_SATA0_CFG_9					0x24
>>> -#define T_SATA0_CFG_9_BASE_ADDRESS_SHIFT		13
>>> +#define T_SATA0_CFG_9_BASE_ADDRESS			0x40020000
>>>
>>> #define SATA_FPCI_BAR5					0x94
>>> -#define SATA_FPCI_BAR5_START_SHIFT			4
>>> +#define SATA_FPCI_BAR5_START_MASK			(0xfffffff << 4)
>>> +#define SATA_FPCI_BAR5_START				(0x0040020 <<
>>> 4)
>>> +#define SATA_FPCI_BAR5_ACCESS_TYPE			(0x1)
>>>
>>> #define SATA_INTR_MASK					0x188
>>> #define SATA_INTR_MASK_IP_INT_MASK			BIT(16)
>>>
>>> +#define T_SATA0_CFG_35					0x94
>>> +#define T_SATA0_CFG_35_IDP_INDEX_MASK			(0x7ff
><< 2)
>>> +#define T_SATA0_CFG_35_IDP_INDEX			(0x2a << 2)
>>> +
>>> +#define T_SATA0_AHCI_IDP1				0x98
>>> +#define T_SATA0_AHCI_IDP1_DATA				(0x400040)
>>> +
>>> +#define T_SATA0_CFG_PHY_1				0x12c
>>> +#define T_SATA0_CFG_PHY_1_PADS_IDDQ_EN			BIT(23)
>>> +#define T_SATA0_CFG_PHY_1_PAD_PLL_IDDQ_EN		BIT(22)
>>> +
>>> +#define T_SATA0_NVOOB                                   0x114
>>> +#define T_SATA0_NVOOB_COMMA_CNT_MASK                    (0xff << 16)
>>> +#define T_SATA0_NVOOB_COMMA_CNT                         (0x07 << 16)
>>> +#define T_SATA0_NVOOB_SQUELCH_FILTER_MODE_MASK          (0x3 << 24)
>>> +#define T_SATA0_NVOOB_SQUELCH_FILTER_MODE               (0x1 << 24)
>>> +#define T_SATA0_NVOOB_SQUELCH_FILTER_LENGTH_MASK        (0x3 << 26)
>>> +#define T_SATA0_NVOOB_SQUELCH_FILTER_LENGTH             (0x3 << 26)
>>> +
>>> +#define T_SATA_CFG_PHY_0                                0x120
>>> +#define T_SATA_CFG_PHY_0_USE_7BIT_ALIGN_DET_FOR_SPD     BIT(11)
>>> +#define T_SATA_CFG_PHY_0_MASK_SQUELCH                   BIT(24)
>>> +
>>> +#define T_SATA0_CFG2NVOOB_2				0x134
>>> +#define T_SATA0_CFG2NVOOB_2_COMWAKE_IDLE_CNT_LOW_MASK
>	(0x1ff
>>> << 18)
>>> +#define T_SATA0_CFG2NVOOB_2_COMWAKE_IDLE_CNT_LOW	(0xc <<
>18)
>>> +
>>> #define T_SATA0_AHCI_HBA_CAP_BKDR			0x300
>>> +#define T_SATA0_AHCI_HBA_CAP_BKDR_PARTIAL_ST_CAP	BIT(13)
>>> +#define T_SATA0_AHCI_HBA_CAP_BKDR_SLUMBER_ST_CAP	BIT(14)
>>> +#define T_SATA0_AHCI_HBA_CAP_BKDR_SALP			BIT(26)
>>> +#define T_SATA0_AHCI_HBA_CAP_BKDR_SUPP_PM		BIT(17)
>>> +#define T_SATA0_AHCI_HBA_CAP_BKDR_SNCQ			BIT(30)
>>>
>>> #define T_SATA0_BKDOOR_CC				0x4a4
>>> +#define T_SATA0_BKDOOR_CC_CLASS_CODE_MASK		(0xffff << 16)
>>> +#define T_SATA0_BKDOOR_CC_CLASS_CODE
>	(0x0106 << 16)
>>> +#define T_SATA0_BKDOOR_CC_PROG_IF_MASK			(0xff <<
>8)
>>> +#define T_SATA0_BKDOOR_CC_PROG_IF			(0x01 << 8)
>>>
>>> #define T_SATA0_CFG_SATA				0x54c
>>> #define T_SATA0_CFG_SATA_BACKDOOR_PROG_IF_EN		BIT(12)
>>> @@ -82,9 +121,35 @@
>>> #define T_SATA0_CHX_PHY_CTRL11				0x6d0
>>> #define T_SATA0_CHX_PHY_CTRL11_GEN2_RX_EQ		(0x2800 << 16)
>>>
>>> +#define T_SATA0_CHX_PHY_CTRL17_0			0x6e8
>>> +#define T_SATA0_CHX_PHY_CTRL17_0_RX_EQ_CTRL_L_GEN1
>	0x55010000
>>> +#define T_SATA0_CHX_PHY_CTRL18_0			0x6ec
>>> +#define T_SATA0_CHX_PHY_CTRL18_0_RX_EQ_CTRL_L_GEN2
>	0x55010000
>>> +#define T_SATA0_CHX_PHY_CTRL20_0			0x6f4
>>> +#define T_SATA0_CHX_PHY_CTRL20_0_RX_EQ_CTRL_H_GEN1	0x1
>>> +#define T_SATA0_CHX_PHY_CTRL21_0			0x6f8
>>> +#define T_SATA0_CHX_PHY_CTRL21_0_RX_EQ_CTRL_H_GEN2	0x1
>>> +
>>> +/* AUX Registers */
>>> +#define SATA_AUX_MISC_CNTL_1_0				0x8
>>> +#define SATA_AUX_MISC_CNTL_1_0_DEVSLP_OVERRIDE		BIT(17)
>>> +#define SATA_AUX_MISC_CNTL_1_0_SDS_SUPPORT		BIT(13)
>>> +#define SATA_AUX_MISC_CNTL_1_0_DESO_SUPPORT		BIT(15)
>>> +
>>> +#define SATA_AUX_RX_STAT_INT_0				0xc
>>> +#define SATA_AUX_RX_STAT_INT_0_SATA_DEVSLP		BIT(7)
>>> +
>>> +#define SATA_AUX_SPARE_CFG0_0				0x18
>>> +#define SATA_AUX_SPARE_CFG0_0_MDAT_TIMER_AFTER_PG_VALID	BIT(14)
>>> +
>>> #define FUSE_SATA_CALIB					0x124
>>> #define FUSE_SATA_CALIB_MASK				0x3
>>>
>>> +enum {
>>> +	NO_DEVSLP	= (1 << 0),
>>> +	NO_DIPM		= (1 << 1),
>>> +};
>>> +
>>> struct sata_pad_calibration {
>>> 	u8 gen1_tx_amp;
>>> 	u8 gen1_tx_peak;
>>> @@ -99,15 +164,89 @@ static const struct sata_pad_calibration
>>> tegra124_pad_calibration[] = {
>>> 	{0x14, 0x0e, 0x1a, 0x0e},
>>> };
>>>
>>> +struct tegra_ahci_ops {
>>> +	int (*init)(struct ahci_host_priv *); };
>>> +
>>> +struct tegra_ahci_soc {
>>> +	const char *const	*supply_names;
>>> +	u32			num_supplies;
>>> +	u32			quirks;
>>> +	struct tegra_ahci_ops	ops;
>>> +};
>>> +
>>> struct tegra_ahci_priv {
>>> 	struct platform_device	   *pdev;
>>> 	void __iomem		   *sata_regs;
>>> +	void __iomem		   *sata_aux_regs;
>>> 	struct reset_control	   *sata_rst;
>>> 	struct reset_control	   *sata_oob_rst;
>>> 	struct reset_control	   *sata_cold_rst;
>>> 	/* Needs special handling, cannot use ahci_platform */
>>> 	struct clk		   *sata_clk;
>>> -	struct regulator_bulk_data supplies[5];
>>> +	struct regulator_bulk_data *supplies;
>>> +	struct tegra_ahci_soc	   *soc_data;
>>> +};
>>> +
>>> +static const char *const tegra124_supply_names[] = {
>>> +	"avdd", "hvdd", "vddio", "target-5v", "target-12v"
>>> +};
>>> +
>>> +static int tegra124_ahci_init(struct ahci_host_priv *hpriv) {
>>> +	struct tegra_ahci_priv *tegra = hpriv->plat_data;
>>> +	struct sata_pad_calibration calib;
>>> +	int ret;
>>> +	u32 val;
>>> +
>>> +	/* Pad calibration */
>>> +	ret = tegra_fuse_readl(FUSE_SATA_CALIB, &val);
>>> +	if (ret)
>>> +		return ret;
>>> +
>>> +	calib = tegra124_pad_calibration[val & FUSE_SATA_CALIB_MASK];
>>> +
>>> +	writel(BIT(0), tegra->sata_regs + SCFG_OFFSET + T_SATA0_INDEX);
>>> +
>>> +	val = readl(tegra->sata_regs +
>>> +		    SCFG_OFFSET + T_SATA0_CHX_PHY_CTRL1_GEN1);
>>> +	val &= ~T_SATA0_CHX_PHY_CTRL1_GEN1_TX_AMP_MASK;
>>> +	val &= ~T_SATA0_CHX_PHY_CTRL1_GEN1_TX_PEAK_MASK;
>>> +	val |= calib.gen1_tx_amp <<
>>> T_SATA0_CHX_PHY_CTRL1_GEN1_TX_AMP_SHIFT;
>>> +	val |= calib.gen1_tx_peak <<
>>> T_SATA0_CHX_PHY_CTRL1_GEN1_TX_PEAK_SHIFT;
>>> +	writel(val, tegra->sata_regs + SCFG_OFFSET +
>>> +	       T_SATA0_CHX_PHY_CTRL1_GEN1);
>>> +
>>> +	val = readl(tegra->sata_regs +
>>> +		    SCFG_OFFSET + T_SATA0_CHX_PHY_CTRL1_GEN2);
>>> +	val &= ~T_SATA0_CHX_PHY_CTRL1_GEN2_TX_AMP_MASK;
>>> +	val &= ~T_SATA0_CHX_PHY_CTRL1_GEN2_TX_PEAK_MASK;
>>> +	val |= calib.gen2_tx_amp <<
>>> T_SATA0_CHX_PHY_CTRL1_GEN1_TX_AMP_SHIFT;
>>> +	val |= calib.gen2_tx_peak <<
>>> T_SATA0_CHX_PHY_CTRL1_GEN1_TX_PEAK_SHIFT;
>>> +	writel(val, tegra->sata_regs + SCFG_OFFSET +
>>> +	       T_SATA0_CHX_PHY_CTRL1_GEN2);
>>> +
>>> +	writel(T_SATA0_CHX_PHY_CTRL11_GEN2_RX_EQ,
>>> +	       tegra->sata_regs + SCFG_OFFSET + T_SATA0_CHX_PHY_CTRL11);
>>> +	writel(T_SATA0_CHX_PHY_CTRL2_CDR_CNTL_GEN1,
>>> +	       tegra->sata_regs + SCFG_OFFSET + T_SATA0_CHX_PHY_CTRL2);
>>> +
>>> +	writel(0, tegra->sata_regs + SCFG_OFFSET + T_SATA0_INDEX);
>>> +
>>> +	return 0;
>>> +}
>>> +
>>> +static const struct tegra_ahci_soc tegra124_ahci_soc_data = {
>>> +	.supply_names = tegra124_supply_names,
>>> +	.num_supplies = ARRAY_SIZE(tegra124_supply_names),
>>> +	.quirks = NO_DIPM | NO_DEVSLP,
>>> +	.ops = {
>>> +		.init = tegra124_ahci_init,
>>> +		},
>
>Indent this "}," one tab less (i.e. to same level as '.ops')
>
Okay.

>>> +};
>>> +
>>> +static const struct tegra_ahci_soc tegra210_ahci_soc_data = {
>>> +	.quirks = NO_DIPM | NO_DEVSLP,
>>> };
>>>
>>> static int tegra_ahci_power_on(struct ahci_host_priv *hpriv) @@
>>> -115,7 +254,7 @@ static int tegra_ahci_power_on(struct ahci_host_priv
>*hpriv)
>>> 	struct tegra_ahci_priv *tegra = hpriv->plat_data;
>>> 	int ret;
>>>
>>> -	ret = regulator_bulk_enable(ARRAY_SIZE(tegra->supplies),
>>> +	ret = regulator_bulk_enable(tegra->soc_data->num_supplies,
>>> 				    tegra->supplies);
>>> 	if (ret)
>>> 		return ret;
>>> @@ -144,8 +283,7 @@ static int tegra_ahci_power_on(struct
>>> ahci_host_priv
>>> *hpriv)
>>> 	tegra_powergate_power_off(TEGRA_POWERGATE_SATA);
>>>
>>> disable_regulators:
>>> -	regulator_bulk_disable(ARRAY_SIZE(tegra->supplies), tegra->supplies);
>>> -
>>> +	regulator_bulk_disable(tegra->soc_data->num_supplies,
>>> +tegra->supplies);
>
>Indent this.
>
This seems to be issue with my outlook. I don't see this issue
when I sent first using git send-email. Will take care of this. 

>>> 	return ret;
>>> }
>>>
>>> @@ -162,97 +300,137 @@ static void tegra_ahci_power_off(struct
>>> ahci_host_priv *hpriv)
>>> 	clk_disable_unprepare(tegra->sata_clk);
>>> 	tegra_powergate_power_off(TEGRA_POWERGATE_SATA);
>>>
>>> -	regulator_bulk_disable(ARRAY_SIZE(tegra->supplies), tegra->supplies);
>>> +	regulator_bulk_disable(tegra->soc_data->num_supplies,
>>> +tegra->supplies);
>
>And this
>
This again seems to be issue with my outlook. Will correct it.
>>> }
>>>
>>> static int tegra_ahci_controller_init(struct ahci_host_priv *hpriv)  {
>>> 	struct tegra_ahci_priv *tegra = hpriv->plat_data;
>>> 	int ret;
>>> -	unsigned int val;
>>> -	struct sata_pad_calibration calib;
>>> +	u32 val;
>>>
>>> 	ret = tegra_ahci_power_on(hpriv);
>>> -	if (ret) {
>>> -		dev_err(&tegra->pdev->dev,
>>> -			"failed to power on AHCI controller: %d\n", ret);
>>> +	if (ret)
>>> 		return ret;
>>> -	}
>>>
>>> +	/*
>>> +	 * Program the following  SATA IPFS registers
>
>Extra space between "following" and "SATA"
>
Okay, will correct it.
>>> +	 * to allow SW accesses to SATA's MMIO Register
>
>Perhaps say "register range"
>
>>> +	 */
>>> +	val = readl(tegra->sata_regs + SATA_FPCI_BAR5);
>>> +	val &= ~(SATA_FPCI_BAR5_START_MASK |
>>> SATA_FPCI_BAR5_ACCESS_TYPE);
>>> +	val |= SATA_FPCI_BAR5_START | SATA_FPCI_BAR5_ACCESS_TYPE;
>>> +	writel(val, tegra->sata_regs + SATA_FPCI_BAR5);
>>> +
>>> +	/* Program the following SATA IPFS register to enable the SATA */
>>> 	val = readl(tegra->sata_regs + SATA_CONFIGURATION_0);
>>> -	val |= SATA_CONFIGURATION_EN_FPCI;
>>> +	val |= SATA_CONFIGURATION_0_EN_FPCI;
>>> 	writel(val, tegra->sata_regs + SATA_CONFIGURATION_0);
>>>
>>> -	/* Pad calibration */
>>> -
>>> -	ret = tegra_fuse_readl(FUSE_SATA_CALIB, &val);
>>> -	if (ret) {
>>> -		dev_err(&tegra->pdev->dev,
>>> -			"failed to read calibration fuse: %d\n", ret);
>>> -		return ret;
>>> -	}
>>> -
>>> -	calib = tegra124_pad_calibration[val & FUSE_SATA_CALIB_MASK];
>>> -
>>> -	writel(BIT(0), tegra->sata_regs + SCFG_OFFSET + T_SATA0_INDEX);
>>> -
>>> -	val = readl(tegra->sata_regs +
>>> -		SCFG_OFFSET + T_SATA0_CHX_PHY_CTRL1_GEN1);
>>> -	val &= ~T_SATA0_CHX_PHY_CTRL1_GEN1_TX_AMP_MASK;
>>> -	val &= ~T_SATA0_CHX_PHY_CTRL1_GEN1_TX_PEAK_MASK;
>>> -	val |= calib.gen1_tx_amp <<
>>> -			T_SATA0_CHX_PHY_CTRL1_GEN1_TX_AMP_SHIFT;
>>> -	val |= calib.gen1_tx_peak <<
>>> -			T_SATA0_CHX_PHY_CTRL1_GEN1_TX_PEAK_SHIFT;
>>> -	writel(val, tegra->sata_regs + SCFG_OFFSET +
>>> -		T_SATA0_CHX_PHY_CTRL1_GEN1);
>>> -
>>> -	val = readl(tegra->sata_regs +
>>> -			SCFG_OFFSET + T_SATA0_CHX_PHY_CTRL1_GEN2);
>>> -	val &= ~T_SATA0_CHX_PHY_CTRL1_GEN2_TX_AMP_MASK;
>>> -	val &= ~T_SATA0_CHX_PHY_CTRL1_GEN2_TX_PEAK_MASK;
>>> -	val |= calib.gen2_tx_amp <<
>>> -			T_SATA0_CHX_PHY_CTRL1_GEN1_TX_AMP_SHIFT;
>>> -	val |= calib.gen2_tx_peak <<
>>> -			T_SATA0_CHX_PHY_CTRL1_GEN1_TX_PEAK_SHIFT;
>>> -	writel(val, tegra->sata_regs + SCFG_OFFSET +
>>> -		T_SATA0_CHX_PHY_CTRL1_GEN2);
>>> -
>>> -	writel(T_SATA0_CHX_PHY_CTRL11_GEN2_RX_EQ,
>>> -		tegra->sata_regs + SCFG_OFFSET + T_SATA0_CHX_PHY_CTRL11);
>>> -	writel(T_SATA0_CHX_PHY_CTRL2_CDR_CNTL_GEN1,
>>> -		tegra->sata_regs + SCFG_OFFSET + T_SATA0_CHX_PHY_CTRL2);
>>> -
>>> -	writel(0, tegra->sata_regs + SCFG_OFFSET + T_SATA0_INDEX);
>>> -
>>> -	/* Program controller device ID */
>>> +	/* Electrical settings for better link stability */
>>> +	val = T_SATA0_CHX_PHY_CTRL17_0_RX_EQ_CTRL_L_GEN1;
>>> +	writel(val, tegra->sata_regs + SCFG_OFFSET +
>>> T_SATA0_CHX_PHY_CTRL17_0);
>>> +	val = T_SATA0_CHX_PHY_CTRL18_0_RX_EQ_CTRL_L_GEN2;
>>> +	writel(val, tegra->sata_regs + SCFG_OFFSET +
>>> T_SATA0_CHX_PHY_CTRL18_0);
>>> +	val = T_SATA0_CHX_PHY_CTRL20_0_RX_EQ_CTRL_H_GEN1;
>>> +	writel(val, tegra->sata_regs + SCFG_OFFSET +
>>> T_SATA0_CHX_PHY_CTRL20_0);
>>> +	val = T_SATA0_CHX_PHY_CTRL21_0_RX_EQ_CTRL_H_GEN2;
>>> +	writel(val, tegra->sata_regs + SCFG_OFFSET +
>>> +T_SATA0_CHX_PHY_CTRL21_0);
>
>Indent
>
>>> +
>>> +	/* For SQUELCH Filter & Gen3 drive getting detected as Gen1 drive
>>> +*/
>>> +
>>> +	val = readl(tegra->sata_regs + SCFG_OFFSET + T_SATA_CFG_PHY_0);
>>> +	val |= T_SATA_CFG_PHY_0_MASK_SQUELCH;
>>> +	val &= ~T_SATA_CFG_PHY_0_USE_7BIT_ALIGN_DET_FOR_SPD;
>>> +	writel(val, tegra->sata_regs + SCFG_OFFSET + T_SATA_CFG_PHY_0);
>>> +
>>> +	val = readl(tegra->sata_regs + SCFG_OFFSET + T_SATA0_NVOOB);
>>> +	val &= ~(T_SATA0_NVOOB_COMMA_CNT_MASK |
>>> +		 T_SATA0_NVOOB_SQUELCH_FILTER_LENGTH_MASK |
>>> +		 T_SATA0_NVOOB_SQUELCH_FILTER_MODE_MASK);
>>> +	val |= (T_SATA0_NVOOB_COMMA_CNT |
>>> +		T_SATA0_NVOOB_SQUELCH_FILTER_LENGTH |
>>> +		T_SATA0_NVOOB_SQUELCH_FILTER_MODE);
>>> +	writel(val, tegra->sata_regs + SCFG_OFFSET + T_SATA0_NVOOB);
>>> +
>>> +	/*
>>> +	 * Change CFG2NVOOB_2_COMWAKE_IDLE_CNT_LOW from 83.3 ns to
>>> 58.8ns
>>> +	 */
>>> +	val = readl(tegra->sata_regs + SCFG_OFFSET + T_SATA0_CFG2NVOOB_2);
>>> +	val &= ~T_SATA0_CFG2NVOOB_2_COMWAKE_IDLE_CNT_LOW_MASK;
>>> +	val |= T_SATA0_CFG2NVOOB_2_COMWAKE_IDLE_CNT_LOW;
>>> +	writel(val, tegra->sata_regs + SCFG_OFFSET + T_SATA0_CFG2NVOOB_2);
>>> +
>>> +	if (tegra->soc_data->ops.init)
>>> +		tegra->soc_data->ops.init(hpriv);
>>> +
>>> +	/*
>>> +	 * Program the following SATA configuration registers
>>> +	 * to initialize SATA
>>> +	 */
>>> +	val = readl(tegra->sata_regs + SCFG_OFFSET + T_SATA0_CFG_1);
>>> +	val |= (T_SATA0_CFG_1_IO_SPACE | T_SATA0_CFG_1_MEMORY_SPACE |
>>> +		T_SATA0_CFG_1_BUS_MASTER | T_SATA0_CFG_1_SERR);
>>> +	writel(val, tegra->sata_regs + SCFG_OFFSET + T_SATA0_CFG_1);
>>> +	val = T_SATA0_CFG_9_BASE_ADDRESS;
>>> +	writel(val, tegra->sata_regs + SCFG_OFFSET + T_SATA0_CFG_9);
>>>
>>> +	/* Program Class Code and Programming interface for SATA */
>>> 	val = readl(tegra->sata_regs + SCFG_OFFSET + T_SATA0_CFG_SATA);
>>> 	val |= T_SATA0_CFG_SATA_BACKDOOR_PROG_IF_EN;
>>> 	writel(val, tegra->sata_regs + SCFG_OFFSET + T_SATA0_CFG_SATA);
>>>
>>> -	writel(0x01060100, tegra->sata_regs + SCFG_OFFSET +
>>> T_SATA0_BKDOOR_CC);
>>> +	val = readl(tegra->sata_regs + SCFG_OFFSET + T_SATA0_BKDOOR_CC);
>>> +	val &=
>>> +	    ~(T_SATA0_BKDOOR_CC_CLASS_CODE_MASK |
>>> +	      T_SATA0_BKDOOR_CC_PROG_IF_MASK);
>>> +	val |= T_SATA0_BKDOOR_CC_CLASS_CODE |
>>> T_SATA0_BKDOOR_CC_PROG_IF;
>>> +	writel(val, tegra->sata_regs + SCFG_OFFSET + T_SATA0_BKDOOR_CC);
>>>
>>> 	val = readl(tegra->sata_regs + SCFG_OFFSET + T_SATA0_CFG_SATA);
>>> 	val &= ~T_SATA0_CFG_SATA_BACKDOOR_PROG_IF_EN;
>>> 	writel(val, tegra->sata_regs + SCFG_OFFSET + T_SATA0_CFG_SATA);
>>>
>>> -	/* Enable IO & memory access, bus master mode */
>>> -
>>> -	val = readl(tegra->sata_regs + SCFG_OFFSET + T_SATA0_CFG_1);
>>> -	val |= T_SATA0_CFG_1_IO_SPACE | T_SATA0_CFG_1_MEMORY_SPACE |
>>> -		T_SATA0_CFG_1_BUS_MASTER | T_SATA0_CFG_1_SERR;
>>> -	writel(val, tegra->sata_regs + SCFG_OFFSET + T_SATA0_CFG_1);
>>> -
>>> -	/* Program SATA MMIO */
>>> -
>>> -	writel(0x10000 << SATA_FPCI_BAR5_START_SHIFT,
>>> -	       tegra->sata_regs + SATA_FPCI_BAR5);
>>> -
>>> -	writel(0x08000 << T_SATA0_CFG_9_BASE_ADDRESS_SHIFT,
>>> -	       tegra->sata_regs + SCFG_OFFSET + T_SATA0_CFG_9);
>>> +	/* Enabling LPM capabilities through Backdoor Programming */
>>> +	val = readl(tegra->sata_regs + SCFG_OFFSET +
>>> T_SATA0_AHCI_HBA_CAP_BKDR);
>>> +	val |= (T_SATA0_AHCI_HBA_CAP_BKDR_PARTIAL_ST_CAP |
>>> +		T_SATA0_AHCI_HBA_CAP_BKDR_SLUMBER_ST_CAP |
>>> +		T_SATA0_AHCI_HBA_CAP_BKDR_SALP |
>>> +		T_SATA0_AHCI_HBA_CAP_BKDR_SUPP_PM);
>>> +	writel(val, tegra->sata_regs + SCFG_OFFSET +
>>> +T_SATA0_AHCI_HBA_CAP_BKDR);
>>> +
>>> +	/* SATA Second Level Clock Gating configuration
>>> +	 * Enabling Gating of Tx/Rx clocks and driving Pad IDDQ and Lane
>>> +	 * IDDQ Signals
>>> +	 */
>>> +	val = readl(tegra->sata_regs + SCFG_OFFSET + T_SATA0_CFG_35);
>>> +	val &= ~T_SATA0_CFG_35_IDP_INDEX_MASK;
>>> +	val |= T_SATA0_CFG_35_IDP_INDEX;
>>> +	writel(val, tegra->sata_regs + SCFG_OFFSET + T_SATA0_CFG_35);
>>> +
>>> +	val = T_SATA0_AHCI_IDP1_DATA;
>>> +	writel(val, tegra->sata_regs + SCFG_OFFSET + T_SATA0_AHCI_IDP1);
>>> +
>>> +	val = readl(tegra->sata_regs + SCFG_OFFSET + T_SATA0_CFG_PHY_1);
>>> +	val |= (T_SATA0_CFG_PHY_1_PADS_IDDQ_EN |
>>> +		T_SATA0_CFG_PHY_1_PAD_PLL_IDDQ_EN);
>>> +	writel(val, tegra->sata_regs + SCFG_OFFSET + T_SATA0_CFG_PHY_1);
>>> +
>>> +	/*
>>> +	 *  Indicate Sata only has the capability to enter DevSleep
>
>Stray space in front
>
Okay, will correct it.
>>> +	 * from slumber link.
>>> +	 */
>>> +
>>> +	val = readl(tegra->sata_aux_regs + SATA_AUX_MISC_CNTL_1_0);
>>> +	val |= SATA_AUX_MISC_CNTL_1_0_DESO_SUPPORT;
>>> +	writel(val, tegra->sata_aux_regs + SATA_AUX_MISC_CNTL_1_0);
>>> +
>>> +	/* Enabling IPFS Clock Gating */
>>> +	val = readl(tegra->sata_regs + SATA_CONFIGURATION_0);
>>> +	val &= ~SATA_CONFIGURATION_0_CLK_OVERRIDE;
>>> +	writel(val, tegra->sata_regs + SATA_CONFIGURATION_0);
>>>
>>> -	/* Unmask SATA interrupts */
>>> +	tegra_ahci_handle_quirks(hpriv);
>>>
>>> 	val = readl(tegra->sata_regs + SATA_INTR_MASK);
>>> 	val |= SATA_INTR_MASK_IP_INT_MASK;
>>> @@ -278,7 +456,7 @@ static struct ata_port_operations
>>> ahci_tegra_port_ops = {
>>> 	.host_stop	= tegra_ahci_host_stop,
>>> };
>>>
>>> -static const struct ata_port_info ahci_tegra_port_info = {
>>> +static struct ata_port_info ahci_tegra_port_info = {
>>> 	.flags		= AHCI_FLAG_COMMON,
>>> 	.pio_mask	= ATA_PIO4,
>>> 	.udma_mask	= ATA_UDMA6,
>>> @@ -286,7 +464,14 @@ static const struct ata_port_info
>>> ahci_tegra_port_info = {  };
>>>
>>> static const struct of_device_id tegra_ahci_of_match[] = {
>>> -	{ .compatible = "nvidia,tegra124-ahci" },
>>> +	{
>>> +		.compatible = "nvidia,tegra124-ahci",
>>> +		.data = &tegra124_ahci_soc_data
>>> +	},
>>> +	{
>>> +		.compatible = "nvidia,tegra210-ahci",
>>> +		.data = &tegra210_ahci_soc_data
>>> +	},
>>> 	{}
>>> };
>>> MODULE_DEVICE_TABLE(of, tegra_ahci_of_match); @@ -295,12 +480,27
>@@
>>> static struct scsi_host_template ahci_platform_sht = {
>>> 	AHCI_SHT(DRV_NAME),
>>> };
>>>
>>> +static void tegra_ahci_handle_quirks(struct ahci_host_priv *hpriv) {
>>> +	struct tegra_ahci_priv *tegra = hpriv->plat_data;
>>> +	u32 val;
>>> +
>>> +	if (tegra->soc_data->quirks & NO_DEVSLP) {
>>> +		val = readl(tegra->sata_aux_regs + SATA_AUX_MISC_CNTL_1_0);
>>> +		val &= ~SATA_AUX_MISC_CNTL_1_0_SDS_SUPPORT;
>>> +		writel(val, tegra->sata_aux_regs + SATA_AUX_MISC_CNTL_1_0);
>>> +	} else if (tegra->soc_data->quirks & NO_DIPM) {
>
>I don't think this should be 'else if', but just two separate regular 'if's?
>
Okay, will correct it.
>>> +		ahci_tegra_port_info.flags |= ATA_FLAG_NO_DIPM;
>
>Looking at the downstream device trees, it looks like DIPM and DEVSLP are still
>disabled for Tegra186 - so why don't we just hardcode these quirks, by always
>writing the MISC_CNTL_1_0 register and just adding ATA_FLAG_NO_DIPM to
>ahci_tegra_port_info's static definition.
>

For Tegra186 the devslp pin is shared with PCIE clk req pin. 
In downstream PCIe driver by default assumes that CLKREQ is present and it owns it. 
So by default devslp is disabled. We verify devslp by making sure that pcie does not 
own this pin. This issue will be fixed in future chips.

>>> +	}
>>> +}
>>> +
>>> static int tegra_ahci_probe(struct platform_device *pdev)  {
>>> 	struct ahci_host_priv *hpriv;
>>> 	struct tegra_ahci_priv *tegra;
>>> 	struct resource *res;
>>> 	int ret;
>>> +	unsigned int i;
>>>
>>> 	hpriv = ahci_platform_get_resources(pdev);
>>> 	if (IS_ERR(hpriv))
>>> @@ -311,13 +511,18 @@ static int tegra_ahci_probe(struct
>>> platform_device
>>> *pdev)
>>> 		return -ENOMEM;
>>>
>>> 	hpriv->plat_data = tegra;
>>> -
>>> 	tegra->pdev = pdev;
>>> +	tegra->soc_data =
>>> +	    (struct tegra_ahci_soc *)of_device_get_match_data(&pdev->dev);
>>>
>>> 	res = platform_get_resource(pdev, IORESOURCE_MEM, 1);
>>> 	tegra->sata_regs = devm_ioremap_resource(&pdev->dev, res);
>>> 	if (IS_ERR(tegra->sata_regs))
>>> 		return PTR_ERR(tegra->sata_regs);
>>> +	res = platform_get_resource(pdev, IORESOURCE_MEM, 2);
>>> +	tegra->sata_aux_regs = devm_ioremap_resource(&pdev->dev, res);
>>> +	if (IS_ERR(tegra->sata_aux_regs))
>>> +		return PTR_ERR(tegra->sata_aux_regs);
>
>Requiring aux_regs would break backwards compatibility for Tegra124 device
>trees. We will have to keep aux_regs optional for Tegra124, and skip writes to
>them if they are not available.
>

As explained in the other patch, In current driver changes we are touching the
devslp register to disable DevSlp which is a part of AUX for both t124 and t210.
By default the devslp is enabled for T124 and T210. (Reset value is 1).
I will add this aux register in T124 node as well.

>>>
>>> 	tegra->sata_rst = devm_reset_control_get(&pdev->dev, "sata");
>>> 	if (IS_ERR(tegra->sata_rst)) {
>>> @@ -343,13 +548,17 @@ static int tegra_ahci_probe(struct
>>> platform_device
>>> *pdev)
>>> 		return PTR_ERR(tegra->sata_clk);
>>> 	}
>>>
>>> -	tegra->supplies[0].supply = "avdd";
>>> -	tegra->supplies[1].supply = "hvdd";
>>> -	tegra->supplies[2].supply = "vddio";
>>> -	tegra->supplies[3].supply = "target-5v";
>>> -	tegra->supplies[4].supply = "target-12v";
>>> +	tegra->supplies = devm_kcalloc(&pdev->dev,
>>> +				       tegra->soc_data->num_supplies,
>>> +				       sizeof(*tegra->supplies), GFP_KERNEL);
>>> +	if (!tegra->supplies)
>>> +		return -ENOMEM;
>>> +
>>> +	for (i = 0; i < tegra->soc_data->num_supplies; i++)
>>> +		tegra->supplies[i].supply = tegra->soc_data->supply_names[i];
>>>
>>> -	ret = devm_regulator_bulk_get(&pdev->dev, ARRAY_SIZE(tegra-
>>>> supplies),
>>> +	ret = devm_regulator_bulk_get(&pdev->dev,
>>> +				      tegra->soc_data->num_supplies,
>>> 				      tegra->supplies);
>>> 	if (ret) {
>>> 		dev_err(&pdev->dev, "Failed to get regulators\n"); @@ -385,5
>>> +594,5 @@ static struct platform_driver tegra_ahci_driver = {
>>> module_platform_driver(tegra_ahci_driver);
>>>
>>> MODULE_AUTHOR("Mikko Perttunen <mperttunen-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>");
>>> -MODULE_DESCRIPTION("Tegra124 AHCI SATA driver");
>>> +MODULE_DESCRIPTION("Tegra AHCI SATA driver");
>>> MODULE_LICENSE("GPL v2");
>>> --
>>> 2.1.4
>>
>> --
>> To unsubscribe from this list: send the line "unsubscribe linux-tegra"
>> in the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org More majordomo
>> info at  http://vger.kernel.org/majordomo-info.html
>>
>
>Thanks for posting!
>
>Mikko

^ permalink raw reply

* RE: [PATCH V3 2/3] dt-bindings: tegra: Add tegra210 AHCI
From: Preetham Chandru @ 2017-05-24  4:56 UTC (permalink / raw)
  To: Mikko Perttunen,
	thierry.reding-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org,
	tj-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org
  Cc: tegra-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	linux-ide-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	Laxman Dewangan,
	preetham260-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org,
	Venu Byravarasu, Pavan Kunapuli,
	linux-tegra-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
In-Reply-To: <8b842356-90ef-8214-7104-ea1f417717cb-/1wQRMveznE@public.gmane.org>



>-----Original Message-----
>From: Mikko Perttunen [mailto:cyndis-/1wQRMveznE@public.gmane.org]
>Sent: Sunday, May 14, 2017 4:56 PM
>To: Preetham Chandru <pchandru-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>; thierry.reding-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org;
>tj-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org
>Cc: tegra-u79uwXL29TY76Z2rM5mHXA@public.gmane.org; linux-ide-u79uwXL29TY76Z2rM5mHXA@public.gmane.org; Laxman Dewangan
><ldewangan-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>; preetham260-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org; Venu Byravarasu
><vbyravarasu-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>; Pavan Kunapuli <pkunapuli-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>; linux-
>tegra-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
>Subject: Re: [PATCH V3 2/3] dt-bindings: tegra: Add tegra210 AHCI
>
>On 05/12/2017 12:55 PM, Preetham Chandru wrote:
>> +CC linux-tegra-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
>>
>>> -----Original Message-----
>>> From: Preetham Chandru
>>> Sent: Friday, May 12, 2017 3:04 PM
>>> To: thierry.reding-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org; tj-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org
>>> Cc: tegra-u79uwXL29TY76Z2rM5mHXA@public.gmane.org; linux-ide-u79uwXL29TY76Z2rM5mHXA@public.gmane.org; Laxman Dewangan
>>> <ldewangan-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>; preetham260-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org; Venu Byravarasu
>>> <vbyravarasu-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>; Pavan Kunapuli <pkunapuli-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>;
>>> Preetham Chandru <pchandru-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>
>>> Subject: [PATCH V3 2/3] dt-bindings: tegra: Add tegra210 AHCI
>>>
>>> From: Preetham Chandru R <pchandru-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>
>>>
>
>This needs a commit message. Something simple, like "This adds bindings
>documentation for the AHCI controller on Tegra210." is fine.
>

Okay, will change it.

>>> Signed-off-by: Preetham Chandru R <pchandru-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>
>>> ---
>>> v3:
>>> * Add AUX register.
>>> v2:
>>> * change cml1, pll_e and phy regulators as optional  for T210.
>>> ---
>>> .../bindings/ata/nvidia,tegra124-ahci.txt          | 45 +++++++++++++++-------
>>> 1 file changed, 31 insertions(+), 14 deletions(-)
>>>
>>> diff --git
>>> a/Documentation/devicetree/bindings/ata/nvidia,tegra124-ahci.txt
>>> b/Documentation/devicetree/bindings/ata/nvidia,tegra124-ahci.txt
>>> index 66c83c3..dc62dba 100644
>>> --- a/Documentation/devicetree/bindings/ata/nvidia,tegra124-ahci.txt
>>> +++ b/Documentation/devicetree/bindings/ata/nvidia,tegra124-ahci.txt
>>> @@ -1,32 +1,49 @@
>>> -Tegra124 SoC SATA AHCI controller
>>> +Tegra SoC SATA AHCI controller
>>>
>>> Required properties :
>>> -- compatible : For Tegra124, must contain "nvidia,tegra124-ahci".
>>> Otherwise,
>>> -  must contain '"nvidia,<chip>-ahci", "nvidia,tegra124-ahci"', where
>>> <chip>
>>> -  is tegra132.
>>> -- reg : Should contain 2 entries:
>>> +- compatible : Must be one of:
>>> +  - Tegra124 : "nvidia,tegra124-ahci"
>>> +  - Tegra210 : "nvidia,tegra210-ahci"
>>> +- reg : Should contain 3 entries:
>>>   - AHCI register set (SATA BAR5)
>>>   - SATA register set
>>> +  - AUX register set
>
>The AUX register set must be optional on Tegra124, as it was not required
>previously.
>

In current driver changes we are touching the devslp register to disable DevSlp
which is a part of AUX for both t124 and t210. By default the devslp is enabled
for T124 and T210. (Reset value is 1). I will add this aux register in T124 node as well.

>>> - interrupts : Defines the interrupt used by SATA
>>> - clocks : Must contain an entry for each entry in clock-names.
>>>   See ../clocks/clock-bindings.txt for details.
>>> - clock-names : Must include the following entries:
>>>   - sata
>>>   - sata-oob
>>> -  - cml1
>>> -  - pll_e
>>> - resets : Must contain an entry for each entry in reset-names.
>>>   See ../reset/reset.txt for details.
>>> - reset-names : Must include the following entries:
>>>   - sata
>>>   - sata-oob
>>> -  - sata-cold
>>> +  - For T124: sata-cold
>>> - phys : Must contain an entry for each entry in phy-names.
>>>   See ../phy/phy-bindings.txt for details.
>>> - phy-names : Must include the following entries:
>>> -  - sata-phy : XUSB PADCTL SATA PHY
>>> -- hvdd-supply : Defines the SATA HVDD regulator
>>> -- vddio-supply : Defines the SATA VDDIO regulator
>>> -- avdd-supply : Defines the SATA AVDD regulator
>>> -- target-5v-supply : Defines the SATA 5V power regulator
>>> -- target-12v-supply : Defines the SATA 12V power regulator
>>> +  - For T124:
>>> +    - sata-phy : XUSB PADCTL SATA PHY
>>> +  - For T210:
>>> +    - sata-0
>>> +- For T124:
>>> +  - hvdd-supply : Defines the SATA HVDD regulator
>>> +  - vddio-supply : Defines the SATA VDDIO regulator
>>> +  - avdd-supply : Defines the SATA AVDD regulator
>>> +  - target-5v-supply : Defines the SATA 5V power regulator
>
>This is missing the 12V supply regulator
>

Okay. Will add.
>>> +
>>> +Optional properties:
>>> +- clock-names :
>>> +  - cml1 :
>>> +    cml1 clock is required by phy so it is optional to define
>>> +    here as phy driver will be enabling this clock.
>>> +  - pll_e :
>>> +    pll_e is the parent of cml1 clock so it is optional to define
>>> +    here as phy driver will be enabling this clock.
>>> +- For T210:
>>> +  - l0-hvddio-sata-supply : Defines the SATA HVDDIO regulator
>>> +  - l0-dvddio-sata-supply : Defines the SATA DVDDIO regulator
>>> +  - hvdd-pex-pll-e-supply : Defines the PEX PLL_E regulator
>>> +  - dvdd-sata-pll-supply  : Defines the SATA PLL regulator
>>> +  - hvdd-sata-supply      : Defines the SATA HVDD regulator
>
>I don't think the driver currently has code to enable these regulators?
>

Yes, they will be enabled in uphy. Will remove here.
>>> --
>>> 2.1.4
>>
>> --
>> To unsubscribe from this list: send the line "unsubscribe linux-tegra"
>> in the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org More majordomo
>> info at  http://vger.kernel.org/majordomo-info.html
>>

^ permalink raw reply

* RE: [PATCH V3 3/3] arm64: tegra: Enable SATA on Tegra210
From: Preetham Chandru @ 2017-05-24  4:43 UTC (permalink / raw)
  To: Mikko Perttunen,
	thierry.reding-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org,
	tj-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org
  Cc: linux-ide-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	Laxman Dewangan,
	preetham260-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org,
	Venu Byravarasu, Pavan Kunapuli,
	linux-tegra-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
In-Reply-To: <3279be02-37b3-7869-4fe6-5bd20f107f38-/1wQRMveznE@public.gmane.org>



>-----Original Message-----
>From: Mikko Perttunen [mailto:cyndis-/1wQRMveznE@public.gmane.org]
>Sent: Sunday, May 14, 2017 4:59 PM
>To: Preetham Chandru <pchandru-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>; thierry.reding-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org;
>tj-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org
>Cc: tegra-u79uwXL29TY76Z2rM5mHXA@public.gmane.org; linux-ide-u79uwXL29TY76Z2rM5mHXA@public.gmane.org; Laxman Dewangan
><ldewangan-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>; preetham260-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org; Venu Byravarasu
><vbyravarasu-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>; Pavan Kunapuli <pkunapuli-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>; linux-
>tegra-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
>Subject: Re: [PATCH V3 3/3] arm64: tegra: Enable SATA on Tegra210
>
>On 05/12/2017 12:56 PM, Preetham Chandru wrote:
>> +CC linux-tegra-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
>>
>>> -----Original Message-----
>>> From: Preetham Chandru
>>> Sent: Friday, May 12, 2017 3:04 PM
>>> To: thierry.reding-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org; tj-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org
>>> Cc: tegra-u79uwXL29TY76Z2rM5mHXA@public.gmane.org; linux-ide-u79uwXL29TY76Z2rM5mHXA@public.gmane.org; Laxman Dewangan
>>> <ldewangan-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>; preetham260-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org; Venu Byravarasu
>>> <vbyravarasu-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>; Pavan Kunapuli <pkunapuli-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>;
>>> Preetham Chandru <pchandru-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>
>>> Subject: [PATCH V3 3/3] arm64: tegra: Enable SATA on Tegra210
>>>
>>> From: Preetham Chandru R <pchandru-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>
>>>
>>> Enable the SATA unit on Tegra210 systems.
>>>
>>> Signed-off-by: Preetham Chandru R <pchandru-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>
>>> ---
>>> arch/arm64/boot/dts/nvidia/tegra210-p2597.dtsi |  6 ++++++
>>> arch/arm64/boot/dts/nvidia/tegra210.dtsi       | 16 ++++++++++++++++
>>> 2 files changed, 22 insertions(+)
>>>
>>> diff --git a/arch/arm64/boot/dts/nvidia/tegra210-p2597.dtsi
>>> b/arch/arm64/boot/dts/nvidia/tegra210-p2597.dtsi
>>> index e5fc67b..58c28b9 100644
>>> --- a/arch/arm64/boot/dts/nvidia/tegra210-p2597.dtsi
>>> +++ b/arch/arm64/boot/dts/nvidia/tegra210-p2597.dtsi
>>> @@ -1324,6 +1324,12 @@
>>> 		status = "okay";
>>> 	};
>>>
>>> +	sata@70020000 {
>>> +		status = "okay";
>>> +		phys = <&{/padctl@7009f000/pads/sata/lanes/sata-0}>;
>>> +		phy-names = "sata-0";
>>> +	};
>>> +
>>> 	padctl@7009f000 {
>>> 		status = "okay";
>>>
>>> diff --git a/arch/arm64/boot/dts/nvidia/tegra210.dtsi
>>> b/arch/arm64/boot/dts/nvidia/tegra210.dtsi
>>> index 8f26c4d..20c9160 100644
>>> --- a/arch/arm64/boot/dts/nvidia/tegra210.dtsi
>>> +++ b/arch/arm64/boot/dts/nvidia/tegra210.dtsi
>>> @@ -795,6 +795,22 @@
>>> 		#iommu-cells = <1>;
>>> 	};
>>>
>>> +	sata@70020000 {
>>> +		compatible = "nvidia,tegra210-ahci";
>>> +		reg = <0x0 0x70027000 0x0 0x2000>, /* AHCI */
>>> +		      <0x0 0x70020000 0x0 0x7000>, /* SATA */
>>> +		      <0x0 0x70001100 0x0 0x1000>; /* SATA AUX*/
>
>Missing space after 'AUX'
>
Okay.

>>> +		interrupts = <GIC_SPI 23 IRQ_TYPE_LEVEL_HIGH>;
>>> +		clocks = <&tegra_car TEGRA210_CLK_SATA>,
>>> +			<&tegra_car TEGRA210_CLK_SATA_OOB>;
>>> +		clock-names = "sata", "sata-oob";
>>> +		resets = <&tegra_car 124>,
>>> +			 <&tegra_car 123>,
>>> +			 <&tegra_car 129>;
>>> +		reset-names = "sata", "sata-oob", "sata-cold";
>
>The binding documentation says that sata-cold is not required for tegra210?
>

It's a mistake in binding documentation. Will correct it in binding doc.

>>> +		status = "disabled";
>>> +	};
>>> +
>>> 	hda@70030000 {
>>> 		compatible = "nvidia,tegra210-hda", "nvidia,tegra30-hda";
>>> 		reg = <0x0 0x70030000 0x0 0x10000>;
>>> --
>>> 2.1.4
>>
>> --
>> To unsubscribe from this list: send the line "unsubscribe linux-tegra"
>> in the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org More majordomo
>> info at  http://vger.kernel.org/majordomo-info.html
>>

^ permalink raw reply

* [PATCH] Revert "ata: sata_mv: Convert to devm_ioremap_resource()"
From: Andrew Lunn @ 2017-05-23 23:39 UTC (permalink / raw)
  To: Tejun Heo
  Cc: Andy Shevchenko, Gregory Clement, Jason Cooper, linux-ide,
	Andrew Lunn

This reverts commit 368e5fbdfc60732643f34f538823ed4bc8829827.

devm_ioremap_resource() enforces that there are no overlapping
resources, where as devm_ioremap() does not. The sata phy driver needs
a subset of the sata IO address space, so maps some of the sata
address space. As a result, sata_mv now fails to probe, reporting it
cannot get its resources, and so we don't have any SATA disks.

Signed-off-by: Andrew Lunn <andrew@lunn.ch>
---
 drivers/ata/sata_mv.c | 13 ++++++++-----
 1 file changed, 8 insertions(+), 5 deletions(-)

diff --git a/drivers/ata/sata_mv.c b/drivers/ata/sata_mv.c
index b66bcda88320..3b2246dded74 100644
--- a/drivers/ata/sata_mv.c
+++ b/drivers/ata/sata_mv.c
@@ -4067,7 +4067,6 @@ static int mv_platform_probe(struct platform_device *pdev)
 	struct ata_host *host;
 	struct mv_host_priv *hpriv;
 	struct resource *res;
-	void __iomem *mmio;
 	int n_ports = 0, irq = 0;
 	int rc;
 	int port;
@@ -4086,9 +4085,8 @@ static int mv_platform_probe(struct platform_device *pdev)
 	 * Get the register base first
 	 */
 	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
-	mmio = devm_ioremap_resource(&pdev->dev, res);
-	if (IS_ERR(mmio))
-		return PTR_ERR(mmio);
+	if (res == NULL)
+		return -EINVAL;
 
 	/* allocate host */
 	if (pdev->dev.of_node) {
@@ -4132,7 +4130,12 @@ static int mv_platform_probe(struct platform_device *pdev)
 	hpriv->board_idx = chip_soc;
 
 	host->iomap = NULL;
-	hpriv->base = mmio - SATAHC0_REG_BASE;
+	hpriv->base = devm_ioremap(&pdev->dev, res->start,
+				   resource_size(res));
+	if (!hpriv->base)
+		return -ENOMEM;
+
+	hpriv->base -= SATAHC0_REG_BASE;
 
 	hpriv->clk = clk_get(&pdev->dev, NULL);
 	if (IS_ERR(hpriv->clk))
-- 
2.11.0


^ permalink raw reply related

* Re: [PATCH] libata: Fix devres handling
From: Tejun Heo @ 2017-05-23 21:27 UTC (permalink / raw)
  To: Linus Walleij; +Cc: Bartlomiej Zolnierkiewicz, linux-ide, stable
In-Reply-To: <20170523211608.GJ13222@htj.duckdns.org>

Hello, again.

On Tue, May 23, 2017 at 05:16:08PM -0400, Tejun Heo wrote:
> That's 96 bytes of difference, which seems too big for devres_node,
> especially on 32bit machines.  Can you check what gdb says on "print
> ((struct devres *)0)->data" or "print sizeof(struct devres_node)"?
> 
> There gotta be something else going on.  devres_alloc() returns the
> data pointer which is the same one which gets passed into the release
> function.

Also, can you please add throw in some printks and see what's being
returned from devres_alloc() and getting set on driver data?  This is
really weird.  I can't think of a reason why they'd differ.

Thanks.

-- 
tejun

^ permalink raw reply

* Re: [PATCH 3/4 v2] ata: Add driver for Faraday Technology FTIDE010
From: Tejun Heo @ 2017-05-23 21:25 UTC (permalink / raw)
  To: Linus Walleij
  Cc: Bartlomiej Zolnierkiewicz, linux-ide, Janos Laube,
	Paulius Zaleckas, linux-arm-kernel, Hans Ulli Kroll,
	Florian Fainelli, John Feng-Hsin Chiang, Greentime Hu
In-Reply-To: <20170519230600.16782-3-linus.walleij@linaro.org>

Hello,

On Sat, May 20, 2017 at 01:05:59AM +0200, Linus Walleij wrote:
> This adds a driver for the Faraday Technology FTIDE010
> PATA IP block.
> 
> When used with the Storlink/Storm/Cortina Systems Gemini
> SoC, the PATA interface is accompanied by a PATA<->SATA
> bridge, so while the device appear as a PATA controller,
> it attaches physically to SATA disks, and also has a
> designated memory area with registers to set up the bridge.
> 
> The Gemini SATA bridge is separated into its own driver
> file to make things modular and make it possible to reuse
> the PATA driver as stand-alone on other systems than the
> Gemini.
> 
> dmesg excerpt from the D-Link DIR-685 storage router:
> gemini-sata-bridge 46000000.sata: SATA ID 00000e00, PHY ID: 01000100
> gemini-sata-bridge 46000000.sata: set up the Gemini IDE/SATA nexus
> ftide010 63000000.ata: set up Gemini PATA0
> ftide010 63000000.ata: device ID 00000500, irq 26, io base 0x63000000
> ftide010 63000000.ata: SATA0 (master) start
> gemini-sata-bridge 46000000.sata: SATA0 PHY ready
> scsi host0: pata-ftide010
> ata1: PATA max UDMA/133 irq 26
> ata1.00: ATA-8: INTEL SSDSA2CW120G3, 4PC10302, max UDMA/133
> ata1.00: 234441648 sectors, multi 1: LBA48 NCQ (depth 0/32)
> ata1.00: configured for UDMA/133
> scsi 0:0:0:0: Direct-Access     ATA      INTEL SSDSA2CW12 0302 PQ: 0 ANSI: 5
> ata1.00: Enabling discard_zeroes_data
> sd 0:0:0:0: [sda] 234441648 512-byte logical blocks: (120 GB/112 GiB)
> sd 0:0:0:0: [sda] Write Protect is off
> sd 0:0:0:0: [sda] Write cache: enabled, read cache:
>                   enabled, doesn't support DPO or FUA
> ata1.00: Enabling discard_zeroes_data
> ata1.00: Enabling discard_zeroes_data
> sd 0:0:0:0: [sda] Attached SCSI disk
> 
> After this I can flawlessly mount and read/write copy etc files
> from /dev/sda[n].

Heh, that's a lot of driver.  Looks okay to me but, Bart, can you
please review and ack this one?

Thanks.

-- 
tejun

^ permalink raw reply

* Re: [PATCH] libata: Fix devres handling
From: Tejun Heo @ 2017-05-23 21:16 UTC (permalink / raw)
  To: Linus Walleij; +Cc: Bartlomiej Zolnierkiewicz, linux-ide, stable
In-Reply-To: <20170519230314.15718-1-linus.walleij@linaro.org>

Hello, Linus.

On Sat, May 20, 2017 at 01:03:14AM +0200, Linus Walleij wrote:
> The ATA hosts are allocated using devres with:
> host = devres_alloc(ata_host_release, sz, GFP_KERNEL);
> However in the ata_host_release() function the host is retrieved
> using dev_get_drvdata() which is not what other devres handlers
> do, instead we should probably use the passed resource.
>
> Before this my kernel crashes badly when I fail to start a host
> in ata_host_start() and need to bail out, because dev_get_drvdata()
> gets the wrong-but-almost-correct pointer (so on some systems it
> may by chance be the right pointer what do I know).
> 
> On ARMv4 Gemini it is not:
> 
> ------------[ cut here ]------------
> WARNING: CPU: 0 PID: 1 at ../lib/refcount.c:184 refcount_sub_and_test+0x9c/0xac
> refcount_t: underflow; use-after-free.
> CPU: 0 PID: 1 Comm: swapper Not tainted 4.12.0-rc1+ #657
> Hardware name: Gemini (Device Tree)
> [<c0010f10>] (unwind_backtrace) from [<c000d8a4>] (show_stack+0x10/0x14)
> [<c000d8a4>] (show_stack) from [<c0018720>] (__warn+0xcc/0xf4)
> [<c0018720>] (__warn) from [<c0018780>] (warn_slowpath_fmt+0x38/0x48)
> [<c0018780>] (warn_slowpath_fmt) from [<c01fffcc>] (refcount_sub_and_test+0x9c/0xac)
> [<c01fffcc>] (refcount_sub_and_test) from [<c01e8a5c>] (kobject_put+0x28/0xe0)
> [<c01e8a5c>] (kobject_put) from [<c029b294>] (ata_host_release+0xb0/0x144)
> [<c029b294>] (ata_host_release) from [<c027326c>] (release_nodes+0x178/0x1fc)
> [<c027326c>] (release_nodes) from [<c02707e4>] (driver_probe_device+0xd0/0x2dc)
> [<c02707e4>] (driver_probe_device) from [<c0270aac>] (__driver_attach+0xbc/0xc0)
> [<c0270aac>] (__driver_attach) from [<c026eeac>] (bus_for_each_dev+0x70/0xa0)
> [<c026eeac>] (bus_for_each_dev) from [<c026f824>] (bus_add_driver+0x178/0x200)
> [<c026f824>] (bus_add_driver) from [<c0271184>] (driver_register+0x78/0xf8)
> [<c0271184>] (driver_register) from [<c05b2d90>] (do_one_initcall+0xac/0x174)
> [<c05b2d90>] (do_one_initcall) from [<c05b2f6c>] (kernel_init_freeable+0x114/0x1cc)
> [<c05b2f6c>] (kernel_init_freeable) from [<c04beeb4>] (kernel_init+0x8/0xf4)
> [<c04beeb4>] (kernel_init) from [<c000a270>] (ret_from_fork+0x14/0x24)
> ---[ end trace 0a4570446a019085 ]---
> 
> Then there is a second (worse) crash when it tries to iterate
> to the next port. But it is all because the host pointer is
> wrong.

This is really weird.  The two can't be different, well, at least
shouldn't.

> In this case, the host should be 0xc7a3f3d0 as it was when it got
> allocated but instead what dev_get_drvdata() returns is 0xc7a3f370.
> Using the passed resource gives the right pointer.

That's 96 bytes of difference, which seems too big for devres_node,
especially on 32bit machines.  Can you check what gdb says on "print
((struct devres *)0)->data" or "print sizeof(struct devres_node)"?

There gotta be something else going on.  devres_alloc() returns the
data pointer which is the same one which gets passed into the release
function.

Thanks.

-- 
tejun

^ permalink raw reply

* Re: [V3,1/3] ata: ahci_tegra: Add AHCI support for tegra210
From: Vagrant Cascadian @ 2017-05-22 18:30 UTC (permalink / raw)
  To: thierry.reding, tj
  Cc: tegra, linux-ide, ldewangan, preetham260, vbyravarasu, pkunapuli,
	Preetham Chandru R
In-Reply-To: <1494581650-11115-2-git-send-email-pchandru@nvidia.com>

[-- Attachment #1: Type: text/plain, Size: 1854 bytes --]

On 2017-05-12, PREETHAM RAMACHANDRA wrote:
> From: Preetham Chandru R <pchandru@nvidia.com>
>
> 1. Move tegra124 specifics to tegra124_ahci_init.
> 2. Separate the regulators needed for tegra124 and tegra210.
> 3. Disable DIPM and Devslp for t210 and t124 as there are known issues

When building with this patch applied on 4.11, I got the following
warnings/errors:

../../../drivers/ata/ahci_tegra.c: In function
‘tegra_ahci_controller_init’:
../../../drivers/ata/ahci_tegra.c:433:2: error: implicit declaration of
function ‘tegra_ahci_handle_quirks’ [-Werror=i\
mplicit-function-declaration]
  tegra_ahci_handle_quirks(hpriv);
  ^~~~~~~~~~~~~~~~~~~~~~~~
../../../drivers/ata/ahci_tegra.c: At top level:
../../../drivers/ata/ahci_tegra.c:483:13: warning: conflicting types for
‘tegra_ahci_handle_quirks’
 static void tegra_ahci_handle_quirks(struct ahci_host_priv *hpriv)
             ^~~~~~~~~~~~~~~~~~~~~~~~
../../../drivers/ata/ahci_tegra.c:483:13: error: static declaration of
‘tegra_ahci_handle_quirks’ follows non-static d\
eclaration
../../../drivers/ata/ahci_tegra.c:433:2: note: previous implicit
declaration of ‘tegra_ahci_handle_quirks’ was here
  tegra_ahci_handle_quirks(hpriv);
  ^~~~~~~~~~~~~~~~~~~~~~~~
../../../drivers/ata/ahci_tegra.c:483:13: warning:
‘tegra_ahci_handle_quirks’ defined but not used [-Wunused-function]
 static void tegra_ahci_handle_quirks(struct ahci_host_priv *hpriv)
             ^~~~~~~~~~~~~~~~~~~~~~~~
cc1: some warnings being treated as errors
/<<PKGBUILDDIR>>/scripts/Makefile.build:305: recipe for target
'drivers/ata/ahci_tegra.o' failed
make[7]: *** [drivers/ata/ahci_tegra.o] Error 1


Full build log:

  https://www.aikidev.net/~vagrant/jetson-tx1/linux_4.11-1~exp3~20170522~1_arm64-2017-05-22T17:54:19Z.build


live well,
  vagrant

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 832 bytes --]

^ permalink raw reply

* (unknown), 
From: mitch_128 @ 2017-05-22 16:10 UTC (permalink / raw)
  To: linux-ide

[-- Attachment #1: 5877262087.zip --]
[-- Type: application/zip, Size: 3116 bytes --]

^ permalink raw reply

* Viable partnership request.
From: Mr Albert Yang @ 2017-05-18 20:52 UTC (permalink / raw)
  To: linux-ide

Compliment of the day,


I have access to very vital information that can be used to move a huge amount of money. 
I have done my homework very well and i have the machineries in place to get it done. 
Ultimately I need an honest person to play an important role in the completion of this business transaction.


Regards,
Albert.


^ permalink raw reply

* (unknown), 
From: mariobronti @ 2017-05-21 11:13 UTC (permalink / raw)
  To: linux-ide

[-- Attachment #1: 37588996.zip --]
[-- Type: application/zip, Size: 39559 bytes --]

^ permalink raw reply

* (unknown), 
From: mgriffit @ 2017-05-20  9:40 UTC (permalink / raw)
  To: linux-ide

[-- Attachment #1: 64749729775761.zip --]
[-- Type: application/zip, Size: 2868 bytes --]

^ permalink raw reply


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox