Linux ATA/IDE development
 help / color / mirror / Atom feed
* Re: [PATCH] libata:pata_atiixp: Don't use unconnected secondary port on SB600/SB700
From: Tejun Heo @ 2017-01-20 20:40 UTC (permalink / raw)
  To: Darren Stevens; +Cc: linux-ide
In-Reply-To: <497611d4655.7b408682@auth.smtp.1and1.co.uk>

Hello, Darren.

On Fri, Jan 20, 2017 at 06:03:32PM +0000, Darren Stevens wrote:
> The SB600 and SB700 southbridge chips from ATI/AMD only have
> connections for the primary IDE port. As these chips have unique
> pci device ID's use these to mark the secondary port as 'dummy'
> 
> Signed-off-by: Darren Stevens <darren@stevens-zone.net>
> ---
> 
> diff --git a/drivers/ata/pata_atiixp.c b/drivers/ata/pata_atiixp.c
> index 49d705c..588c473 100644
> --- a/drivers/ata/pata_atiixp.c
> +++ b/drivers/ata/pata_atiixp.c
> @@ -278,6 +278,11 @@ static int atiixp_init_one(struct pci_dev *pdev, const
> struct pci_device_id *id)
>     };
>     const struct ata_port_info *ppi[] = { &info, &info };
>  
> +   if((pdev->device == PCI_DEVICE_ID_ATI_IXP600_IDE) ||
        ^
	space here

> +       (pdev->device == PCI_DEVICE_ID_ATI_IXP700_IDE))
> +       /* SB600/700 don't have secondary port wired */

Can you move the comment above if?

> +       ppi[1] = &ata_dummy_port_info;
> +
>     return ata_pci_bmdma_init_one(pdev, ppi, &atiixp_sht, NULL,
>                       ATA_HOST_PARALLEL_SCAN);

And the patch is corrupt.  Attaching the formatted patch might work
better.

Thanks.

-- 
tejun

^ permalink raw reply

* Re: [PATCH] pata_legacy: Allow disabling of legacy PATA device probes on non-PCI systems
From: Tejun Heo @ 2017-01-20 19:38 UTC (permalink / raw)
  To: gwendal
  Cc: One Thousand Gnomes, Greg Kroah-Hartman, Sergei Shtylyov,
	linux-ide, whiteheadm
In-Reply-To: <20170120191946.GA9280@mtj.duckdns.org>

Hello, Gwendal.

On Fri, Jan 20, 2017 at 02:19:46PM -0500, Tejun Heo wrote:
> On Fri, Jan 20, 2017 at 12:08:10PM -0500, tedheadster wrote:
> > kobject_get(pata_legacy.1): ref=3++ (ata_tport_add:ata_host_register:ata_host_activate)
> > kobject_get(pata_legacy.1): ref=4++ (ata_tport_add:ata_host_register:ata_host_activate)
> > XXX kobject_get(pata_legacy.1): ref=5++ (kobject_add:device_add:ata_tport_add)
> > XXX kobject_put(pata_legacy.1): ref=6-- (device_del:ata_tport_delete:ata_host_detach)
> > XXX kobject_put(pata_legacy.1): ref=5-- (ata_tport_delete:ata_host_detach:legacy_init [pata_legacy])
> > XXX kobject_put(pata_legacy.1): ref=4-- (klist_put:klist_del:device_del)
> > XXX kobject_put(pata_legacy.1): ref=3-- (klist_devices_put:klist_put:klist_del)
> > XXX kobject_put(pata_legacy.1): ref=2-- (platform_device_unregister:legacy_init [pata_legacy]:do_one_initcall)
> 
> So, three gets from the transport path but only two puts.  That seems
> to be the culprit.  Looking into it.

So, Matthew discovered that pata_legacy isn't releasing resources
after probe failure.  After some debugging, it turns out that the
transport code is taking three refs but only putting two preventing
the ata port from going away.

Can you please explain why the libata transport code is explicitly
pinning the parent device and then putting it in the release methods?
device_add/del() already gets and puts the parent, so I don't get why
this part is necessary.  Is this intentional?

Also, it looks like there is a ref leak on the transport device
itself.  Its release function never gets called and thus the parent
device (ata_port) stays pinned too.

Thanks.

-- 
tejun

^ permalink raw reply

* Re: [PATCH] pata_legacy: Allow disabling of legacy PATA device probes on non-PCI systems
From: Tejun Heo @ 2017-01-20 19:19 UTC (permalink / raw)
  To: whiteheadm
  Cc: One Thousand Gnomes, Greg Kroah-Hartman, Sergei Shtylyov,
	linux-ide
In-Reply-To: <CAP8WD_Y1HmuMots4HcQ06gQfs0fCh1W_y=Qa0As3MX1YA+LEKg@mail.gmail.com>

Hello,

Thanks for testing.

On Fri, Jan 20, 2017 at 12:08:10PM -0500, tedheadster wrote:
> kobject_get(pata_legacy.1): ref=3++ (ata_tport_add:ata_host_register:ata_host_activate)
> kobject_get(pata_legacy.1): ref=4++ (ata_tport_add:ata_host_register:ata_host_activate)
> XXX kobject_get(pata_legacy.1): ref=5++ (kobject_add:device_add:ata_tport_add)
> XXX kobject_put(pata_legacy.1): ref=6-- (device_del:ata_tport_delete:ata_host_detach)
> XXX kobject_put(pata_legacy.1): ref=5-- (ata_tport_delete:ata_host_detach:legacy_init [pata_legacy])
> XXX kobject_put(pata_legacy.1): ref=4-- (klist_put:klist_del:device_del)
> XXX kobject_put(pata_legacy.1): ref=3-- (klist_devices_put:klist_put:klist_del)
> XXX kobject_put(pata_legacy.1): ref=2-- (platform_device_unregister:legacy_init [pata_legacy]:do_one_initcall)

So, three gets from the transport path but only two puts.  That seems
to be the culprit.  Looking into it.

Thanks.

-- 
tejun

^ permalink raw reply

* [PATCH] libata:pata_atiixp: Don't use unconnected secondary port on SB600/SB700
From: Darren Stevens @ 2017-01-20 18:03 UTC (permalink / raw)
  To: linux-ide

The SB600 and SB700 southbridge chips from ATI/AMD only have
connections for the primary IDE port. As these chips have unique
pci device ID's use these to mark the secondary port as 'dummy'

Signed-off-by: Darren Stevens <darren@stevens-zone.net>
---

diff --git a/drivers/ata/pata_atiixp.c b/drivers/ata/pata_atiixp.c
index 49d705c..588c473 100644
--- a/drivers/ata/pata_atiixp.c
+++ b/drivers/ata/pata_atiixp.c
@@ -278,6 +278,11 @@ static int atiixp_init_one(struct pci_dev *pdev, const
struct pci_device_id *id)
    };
    const struct ata_port_info *ppi[] = { &info, &info };
 
+   if((pdev->device == PCI_DEVICE_ID_ATI_IXP600_IDE) ||
+       (pdev->device == PCI_DEVICE_ID_ATI_IXP700_IDE))
+       /* SB600/700 don't have secondary port wired */
+       ppi[1] = &ata_dummy_port_info;
+
    return ata_pci_bmdma_init_one(pdev, ppi, &atiixp_sht, NULL,
                      ATA_HOST_PARALLEL_SCAN);
 }


^ permalink raw reply related

* [PATCH] libata-sff: Don't scan disabled ports when checking for legacy mode.
From: Darren Stevens @ 2017-01-20 17:58 UTC (permalink / raw)
  To: linux-ide

libata-sff.c checks for legacy mode by testing if both primary
and secondary ports on a controller are in legacy mode and selects
legacy if either one is. However on some southbridge chips (e.g
AMD SB600/SB700) the secondary port is not wired, and when it is 
disabled by setting the disable bit in the PCI header it appears
as a fixed legacy port.
Prevent incorrect detection by not testing ports that are marked
as 'dummy'

Signed-off-by: Darren Stevens <darren@stevens-zone.net>
---

diff --git a/drivers/ata/libata-sff.c b/drivers/ata/libata-sff.c
index 051b615..05f688a 100644
--- a/drivers/ata/libata-sff.c
+++ b/drivers/ata/libata-sff.c
@@ -2429,9 +2429,16 @@ int ata_pci_sff_activate_host(struct ata_host *host,
    if ((pdev->class >> 8) == PCI_CLASS_STORAGE_IDE) {
        u8 tmp8, mask;
 
-       /* TODO: What if one channel is in native mode ... */
+       /*
+        * ATA spec says we should use legacy mode when one
+        * port is in legacy mode, but disabled ports on some
+        * PCI hosts appear as fixed legacy ports, e.g SB600/700
+        * on which the secondary port is not wired, so
+        * ignore ports that we've marked as 'dummy' during
+        * this check
+        */
        pci_read_config_byte(pdev, PCI_CLASS_PROG, &tmp8);
-       mask = (1 << 2) | (1 << 0);
+       mask = (! ata_port_is_dummy(host->ports[1]) << 2) | (!
ata_port_is_dummy(host->ports[0]) << 0);
        if ((tmp8 & mask) != mask)
            legacy_mode = 1;
    }


^ permalink raw reply related

* Re: [PATCH] pata_legacy: Allow disabling of legacy PATA device probes on non-PCI systems
From: tedheadster @ 2017-01-20 17:08 UTC (permalink / raw)
  To: Tejun Heo
  Cc: One Thousand Gnomes, Greg Kroah-Hartman, Sergei Shtylyov,
	linux-ide
In-Reply-To: <20170119213737.GB25133@mtj.duckdns.org>

Tejun,
  here is the relevant output:

[   39.868755] XXX kobject_get(pata_legacy.0): ref=3++
(ata_tport_add:ata_host_register:ata_host_activate)
[   39.875762] XXX kobject_get(pata_legacy.0): ref=4++
(ata_tport_add:ata_host_register:ata_host_activate)
[   39.877885] XXX kobject_get(pata_legacy.0): ref=5++
(kobject_add:device_add:ata_tport_add)
[   39.971742] scsi host0: pata_legacy
[   39.999736] ata1: PATA max PIO4 cmd 0x1f0 ctl 0x3f6 irq 14
[   40.157745] ata1.00: ATA-4: QUANTUM FIREBALL CR8.4A, A5U.1200, max UDMA/66
[   40.159149] ata1.00: 16514064 sectors, multi 0: LBA
[   40.160111] ata1.00: configured for PIO
[   40.236700] scsi 0:0:0:0: Direct-Access     ATA      QUANTUM
FIREBALL 1200 PQ: 0 ANSI: 5
[   40.337686] sd 0:0:0:0: [sda] 16514064 512-byte logical blocks:
(8.46 GB/7.87 GiB)
[   40.346688] sd 0:0:0:0: [sda] Write Protect is off
[   40.348681] sd 0:0:0:0: [sda] Mode Sense: 00 3a 00 00
[   40.365678] sd 0:0:0:0: [sda] Write cache: enabled, read cache:
enabled, doesn't support DPO or FUA
[   40.367688] sd 0:0:0:0: Attached scsi generic sg0 type 0
[   40.540654]  sda: sda1 sda2 sda3 sda4
[   40.916596] sd 0:0:0:0: [sda] Attached SCSI disk
[   40.956594] XXX kobject_get(pata_legacy.1): ref=3++
(ata_tport_add:ata_host_register:ata_host_activate)
[   40.968587] XXX kobject_get(pata_legacy.1): ref=4++
(ata_tport_add:ata_host_register:ata_host_activate)
[   40.971139] XXX kobject_get(pata_legacy.1): ref=5++
(kobject_add:device_add:ata_tport_add)
[   41.152561] scsi host1: pata_legacy
[   41.227549] ata2: PATA max PIO4 cmd 0x170 ctl 0x376 irq 15
[   41.690486] XXX kobject_put(pata_legacy.1): ref=6--
(device_del:ata_tport_delete:ata_host_detach)
[   41.692548] XXX kobject_put(pata_legacy.1): ref=5--
(ata_tport_delete:ata_host_detach:legacy_init [pata_legacy])
[   41.719476] XXX kobject_put(pata_legacy.1): ref=4--
(klist_put:klist_del:device_del)
[   41.725471] XXX kobject_put(pata_legacy.1): ref=3--
(klist_devices_put:klist_put:klist_del)
[   41.745472] XXX kobject_put(pata_legacy.1): ref=2--
(platform_device_unregister:legacy_init [pata_legacy]:do_one_initcall)
[   41.793466] XXX kobject_get(pata_legacy.2): ref=3++
(ata_tport_add:ata_host_register:ata_host_activate)
[   41.807475] XXX kobject_get(pata_legacy.2): ref=4++
(ata_tport_add:ata_host_register:ata_host_activate)
[   41.809848] XXX kobject_get(pata_legacy.2): ref=5++
(kobject_add:device_add:ata_tport_add)
[   41.991431] scsi host2: pata_legacy
[   42.036428] ata3: PATA max PIO4 cmd 0x1e8 ctl 0x3ee irq 11
[   42.357388] XXX kobject_put(pata_legacy.2): ref=6--
(device_del:ata_tport_delete:ata_host_detach)
[   42.359786] XXX kobject_put(pata_legacy.2): ref=5--
(ata_tport_delete:ata_host_detach:legacy_init [pata_legacy])
[   42.447367] XXX kobject_put(pata_legacy.2): ref=4--
(klist_put:klist_del:device_del)
[   42.470362] XXX kobject_put(pata_legacy.2): ref=3--
(klist_devices_put:klist_put:klist_del)
[   42.510357] XXX kobject_put(pata_legacy.2): ref=2--
(platform_device_unregister:legacy_init [pata_legacy]:do_one_initcall)
[   42.583346] XXX kobject_get(pata_legacy.3): ref=3++
(ata_tport_add:ata_host_register:ata_host_activate)
[   42.592340] XXX kobject_get(pata_legacy.3): ref=4++
(ata_tport_add:ata_host_register:ata_host_activate)
[   42.594415] XXX kobject_get(pata_legacy.3): ref=5++
(kobject_add:device_add:ata_tport_add)
[   42.815304] scsi host3: pata_legacy
[   42.881302] ata4: PATA max PIO4 cmd 0x168 ctl 0x36e irq 10
[   43.292234] XXX kobject_put(pata_legacy.3): ref=6--
(device_del:ata_tport_delete:ata_host_detach)
[   43.294228] XXX kobject_put(pata_legacy.3): ref=5--
(ata_tport_delete:ata_host_detach:legacy_init [pata_legacy])
[   43.340228] XXX kobject_put(pata_legacy.3): ref=4--
(klist_put:klist_del:device_del)
[   43.370225] XXX kobject_put(pata_legacy.3): ref=3--
(klist_devices_put:klist_put:klist_del)
[   43.411220] XXX kobject_put(pata_legacy.3): ref=2--
(platform_device_unregister:legacy_init [pata_legacy]:do_one_initcall)
[   43.599193] genirq: Flags mismatch irq 8. 00000000
(platform[pata_legacy.4]) vs. 00000080 (rtc0)
[   43.645179] XXX kobject_put(pata_legacy.4): ref=3--
(klist_put:klist_del:device_del)
[   43.685178] XXX kobject_put(pata_legacy.4): ref=2--
(klist_devices_put:klist_put:klist_del)
[   43.733174] XXX kobject_put(pata_legacy.4): ref=1--
(platform_device_unregister:legacy_init [pata_legacy]:do_one_initcall)
[   43.736259] XXX kobject_cleanup(pata_legacy.4): rel=device_release
(put_device:platform_device_unregister:legacy_init [pata_legacy])
[   43.738417] platform pata_legacy.4: XXX device_release:
drel=platform_device_release trel=  (null) cdrel=  (null)
[   43.740160] platform pata_legacy.4: XXX device_release:
devres_release_all() done
[   43.950138] XXX kobject_get(pata_legacy.5): ref=3++
(ata_tport_add:ata_host_register:ata_host_activate)
[   43.973137] XXX kobject_get(pata_legacy.5): ref=4++
(ata_tport_add:ata_host_register:ata_host_activate)
[   43.975360] XXX kobject_get(pata_legacy.5): ref=5++
(kobject_add:device_add:ata_tport_add)
[   44.338072] scsi host4: pata_legacy
[   44.389071] ata5: PATA max PIO4 cmd 0x160 ctl 0x366 irq 12
[   44.699020] XXX kobject_put(pata_legacy.5): ref=6--
(device_del:ata_tport_delete:ata_host_detach)
[   44.701270] XXX kobject_put(pata_legacy.5): ref=5--
(ata_tport_delete:ata_host_detach:legacy_init [pata_legacy])
[   44.714021] XXX kobject_put(pata_legacy.5): ref=4--
(klist_put:klist_del:device_del)
[   44.731015] XXX kobject_put(pata_legacy.5): ref=3--
(klist_devices_put:klist_put:klist_del)
[   44.750009] XXX kobject_put(pata_legacy.5): ref=2--
(platform_device_unregister:legacy_init [pata_legacy]:do_one_initcall)

...

[  252.348369] genirq: Flags mismatch irq 10. 00000000 (3c515) vs.
00000000 (platform[pata_legacy.3])
[  252.379341] genirq: Flags mismatch irq 10. 00000000 (3c515) vs.
00000000 (platform[pata_legacy.3])

- Matthew

^ permalink raw reply

* Re: [PATCH v5 00/14] ARM: da850-lcdk: add SATA support
From: Tejun Heo @ 2017-01-20 15:00 UTC (permalink / raw)
  To: Sekhar Nori
  Cc: Bartosz Golaszewski, Kevin Hilman, Patrick Titiano,
	Michael Turquette, Rob Herring, Mark Rutland, Russell King,
	David Lechner, linux-ide-u79uwXL29TY76Z2rM5mHXA, linux-devicetree,
	LKML, arm-soc, Olof Johansson, Arnd Bergmann
In-Reply-To: <f9055043-507f-74de-b2b8-9eb0eae5dad6-l0cyMroinI0@public.gmane.org>

Hello, Sekhar.

On Fri, Jan 20, 2017 at 07:52:35PM +0530, Sekhar Nori wrote:
> Tejun, I am open to queuing the driver changes through ARM-SoC. I guess
> with that there is little chance that SATA will be broken on linux-next
> for a longish period of time.

Yeah, I'd prefer the patchset staying together.  That's how it was
developed and tested.  Don't wanna break it apart unnecessarily.  The
chance for conflicts is low and even when that happens dealing with
them is pretty easy.  Please feel free to add my acked-by and route
the patches through ARM-SoC.

Thanks.

-- 
tejun
--
To unsubscribe from this list: send the line "unsubscribe devicetree" 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 v5 00/14] ARM: da850-lcdk: add SATA support
From: Sekhar Nori @ 2017-01-20 14:22 UTC (permalink / raw)
  To: Bartosz Golaszewski
  Cc: Tejun Heo, Kevin Hilman, Patrick Titiano, Michael Turquette,
	Rob Herring, Mark Rutland, Russell King, David Lechner, linux-ide,
	linux-devicetree, LKML, arm-soc, Olof Johansson, Arnd Bergmann
In-Reply-To: <CAMpxmJUPh9MZKwG6k6p_mo6niXNNT02JCDgXi4DPRH2Fk3yxng@mail.gmail.com>

On Friday 20 January 2017 07:32 PM, Bartosz Golaszewski wrote:
> 2017-01-20 14:56 GMT+01:00 Sekhar Nori <nsekhar@ti.com>:
>> Hi Tejun,
>>
>> On Friday 20 January 2017 06:58 PM, Tejun Heo wrote:
>>> On Fri, Jan 20, 2017 at 12:21:51PM +0100, Bartosz Golaszewski wrote:
>>>> This series contains all the changes necessary to make SATA work on
>>>> the da850-lcdk board.
>>>>
>>>> The first patch adds DT bindings for the ahci-da850 driver.
>>>>
>>>> The second enables relevant modules in davinci_all_defconfig.
>>>>
>>>> Patches 03/14-06/14 modify the way the clocks are handled regarding
>>>> SATA on the da850 platform. We modify the ahci driver to retrieve
>>>> the clock via con_id and model the external SATA oscillator as
>>>> a real clock.
>>>>
>>>> Patches 07/14-11/14 extend the ahci-da850 driver. Add DT support,
>>>> implement workarounds necessary to make SATA work on the da850-lcdk
>>>> board and un-hardcode the external clock multiplier.
>>>
>>> Please feel free to add
>>>
>>>  Acked-by: Tejun Heo <tj@kernel.org>
>>>
>>> to the all libata patches.  Please let me know how the patches should
>>> be routed once other parts are settled.
>>
>> I believe you can queue the libata patches independently (patches 1, 4,
>> 7, 8, 9, 10, 11). It looks like they have been written such that driver
>> continues to work with existing platform code (Bartosz, please disagree
>> if I am wrong). 1/14 still needs the ack from DT maintainers.
>>
> 
> Patch 11/14 depends on 06/14. Other than that I think it should work
> independently.

Ah, right. You recently changed 11/14 to error out in case refclkpn is
not found instead of using a default value (do agree that current
version is better).

If there are no build time dependencies still, it might be okay to queue
these patches through their respective maintainer trees.

Tejun, I am open to queuing the driver changes through ARM-SoC. I guess
with that there is little chance that SATA will be broken on linux-next
for a longish period of time.

I have Cced the ARM-SoC maintainers too.

Thanks,
Sekhar

^ permalink raw reply

* Re: [PATCH v5 00/14] ARM: da850-lcdk: add SATA support
From: Bartosz Golaszewski @ 2017-01-20 14:02 UTC (permalink / raw)
  To: Sekhar Nori
  Cc: Mark Rutland, linux-devicetree, David Lechner, Kevin Hilman,
	Michael Turquette, Russell King, LKML, linux-ide, Rob Herring,
	Patrick Titiano, Tejun Heo, arm-soc
In-Reply-To: <86bcbb3c-87e4-3508-f4a6-26529f39d374@ti.com>

2017-01-20 14:56 GMT+01:00 Sekhar Nori <nsekhar@ti.com>:
> Hi Tejun,
>
> On Friday 20 January 2017 06:58 PM, Tejun Heo wrote:
>> On Fri, Jan 20, 2017 at 12:21:51PM +0100, Bartosz Golaszewski wrote:
>>> This series contains all the changes necessary to make SATA work on
>>> the da850-lcdk board.
>>>
>>> The first patch adds DT bindings for the ahci-da850 driver.
>>>
>>> The second enables relevant modules in davinci_all_defconfig.
>>>
>>> Patches 03/14-06/14 modify the way the clocks are handled regarding
>>> SATA on the da850 platform. We modify the ahci driver to retrieve
>>> the clock via con_id and model the external SATA oscillator as
>>> a real clock.
>>>
>>> Patches 07/14-11/14 extend the ahci-da850 driver. Add DT support,
>>> implement workarounds necessary to make SATA work on the da850-lcdk
>>> board and un-hardcode the external clock multiplier.
>>
>> Please feel free to add
>>
>>  Acked-by: Tejun Heo <tj@kernel.org>
>>
>> to the all libata patches.  Please let me know how the patches should
>> be routed once other parts are settled.
>
> I believe you can queue the libata patches independently (patches 1, 4,
> 7, 8, 9, 10, 11). It looks like they have been written such that driver
> continues to work with existing platform code (Bartosz, please disagree
> if I am wrong). 1/14 still needs the ack from DT maintainers.
>

Patch 11/14 depends on 06/14. Other than that I think it should work
independently.

Thanks,
Bartosz

^ permalink raw reply

* Re: [PATCH v5 00/14] ARM: da850-lcdk: add SATA support
From: Sekhar Nori @ 2017-01-20 13:56 UTC (permalink / raw)
  To: Tejun Heo, Bartosz Golaszewski
  Cc: Mark Rutland, devicetree, David Lechner, Kevin Hilman,
	Michael Turquette, Russell King, linux-kernel, linux-ide,
	Rob Herring, Patrick Titiano, linux-arm-kernel
In-Reply-To: <20170120132843.GA31487@mtj.duckdns.org>

Hi Tejun,

On Friday 20 January 2017 06:58 PM, Tejun Heo wrote:
> On Fri, Jan 20, 2017 at 12:21:51PM +0100, Bartosz Golaszewski wrote:
>> This series contains all the changes necessary to make SATA work on
>> the da850-lcdk board.
>>
>> The first patch adds DT bindings for the ahci-da850 driver.
>>
>> The second enables relevant modules in davinci_all_defconfig.
>>
>> Patches 03/14-06/14 modify the way the clocks are handled regarding
>> SATA on the da850 platform. We modify the ahci driver to retrieve
>> the clock via con_id and model the external SATA oscillator as
>> a real clock.
>>
>> Patches 07/14-11/14 extend the ahci-da850 driver. Add DT support,
>> implement workarounds necessary to make SATA work on the da850-lcdk
>> board and un-hardcode the external clock multiplier.
> 
> Please feel free to add
> 
>  Acked-by: Tejun Heo <tj@kernel.org>
> 
> to the all libata patches.  Please let me know how the patches should
> be routed once other parts are settled.

I believe you can queue the libata patches independently (patches 1, 4,
7, 8, 9, 10, 11). It looks like they have been written such that driver
continues to work with existing platform code (Bartosz, please disagree
if I am wrong). 1/14 still needs the ack from DT maintainers.

I would like to queue the platform parts (rest of the patches) through
ARM-SoC since they will likely clash with code I have queued.

Thanks,
Sekhar

^ permalink raw reply

* Re: [PATCH 1/3 v2] ahci: qoriq: added a condition to enable dma coherence
From: Tejun Heo @ 2017-01-20 13:34 UTC (permalink / raw)
  To: yuantian.tang
  Cc: mark.rutland, devicetree, mathieu.poirier, linux-kernel,
	linux-ide, robh+dt, robin.murphy, linux-arm-kernel
In-Reply-To: <1484895576-40379-1-git-send-email-yuantian.tang@nxp.com>

On Fri, Jan 20, 2017 at 02:59:34PM +0800, yuantian.tang@nxp.com wrote:
> From: Tang Yuantian <Yuantian.Tang@nxp.com>
> 
> Enable DMA coherence in SATA controller on condition that
> dma-coherent property exists in sata node in DTS.
> 
> Signed-off-by: Tang Yuantian <yuantian.tang@nxp.com>

Applied to libata/for-4.11.

Thanks.

-- 
tejun

^ permalink raw reply

* Re: [PATCH v5 01/14] devicetree: bindings: add bindings for ahci-da850
From: Sekhar Nori @ 2017-01-20 13:32 UTC (permalink / raw)
  To: Bartosz Golaszewski, Kevin Hilman, Patrick Titiano,
	Michael Turquette, Tejun Heo, Rob Herring, Mark Rutland,
	Russell King, David Lechner
  Cc: linux-ide, devicetree, linux-kernel, linux-arm-kernel
In-Reply-To: <1484911325-23425-2-git-send-email-bgolaszewski@baylibre.com>

Hi Rob,

On Friday 20 January 2017 04:51 PM, Bartosz Golaszewski wrote:
> Add DT bindings for the TI DA850 AHCI SATA controller.
> 
> Signed-off-by: Bartosz Golaszewski <bgolaszewski@baylibre.com>

Could you ack this binding?

Thanks,
Sekhar

> ---
>  Documentation/devicetree/bindings/ata/ahci-da850.txt | 15 +++++++++++++++
>  1 file changed, 15 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/ata/ahci-da850.txt
> 
> diff --git a/Documentation/devicetree/bindings/ata/ahci-da850.txt b/Documentation/devicetree/bindings/ata/ahci-da850.txt
> new file mode 100644
> index 0000000..268fe87
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/ata/ahci-da850.txt
> @@ -0,0 +1,15 @@
> +Device tree binding for the TI DA850 AHCI SATA Controller
> +---------------------------------------------------------
> +
> +Required properties:
> +  - compatible: must be "ti,da850-ahci"
> +  - reg: physical base addresses and sizes of the controller's register areas
> +  - interrupts: interrupt specifier (refer to the interrupt binding)
> +
> +Example:
> +
> +	sata: sata@218000 {
> +		compatible = "ti,da850-ahci";
> +		reg = <0x218000 0x2000>, <0x22c018 0x4>;
> +		interrupts = <67>;
> +	};
> 

^ permalink raw reply

* Re: [PATCH v5 00/14] ARM: da850-lcdk: add SATA support
From: Tejun Heo @ 2017-01-20 13:28 UTC (permalink / raw)
  To: Bartosz Golaszewski
  Cc: Kevin Hilman, Sekhar Nori, Patrick Titiano, Michael Turquette,
	Rob Herring, Mark Rutland, Russell King, David Lechner,
	linux-ide-u79uwXL29TY76Z2rM5mHXA,
	devicetree-u79uwXL29TY76Z2rM5mHXA,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA,
	linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r
In-Reply-To: <1484911325-23425-1-git-send-email-bgolaszewski-rdvid1DuHRBWk0Htik3J/w@public.gmane.org>

On Fri, Jan 20, 2017 at 12:21:51PM +0100, Bartosz Golaszewski wrote:
> This series contains all the changes necessary to make SATA work on
> the da850-lcdk board.
> 
> The first patch adds DT bindings for the ahci-da850 driver.
> 
> The second enables relevant modules in davinci_all_defconfig.
> 
> Patches 03/14-06/14 modify the way the clocks are handled regarding
> SATA on the da850 platform. We modify the ahci driver to retrieve
> the clock via con_id and model the external SATA oscillator as
> a real clock.
> 
> Patches 07/14-11/14 extend the ahci-da850 driver. Add DT support,
> implement workarounds necessary to make SATA work on the da850-lcdk
> board and un-hardcode the external clock multiplier.

Please feel free to add

 Acked-by: Tejun Heo <tj-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>

to the all libata patches.  Please let me know how the patches should
be routed once other parts are settled.

Thanks.

-- 
tejun
--
To unsubscribe from this list: send the line "unsubscribe devicetree" 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 v2] ata: xgene: Enable NCQ support for APM X-Gene SATA controller hardware v1.1
From: Rameshwar Prasad Sahu @ 2017-01-20 12:11 UTC (permalink / raw)
  To: olof, tj, arnd
  Cc: devicetree, mlangsdo, linux-scsi, jcm, Rameshwar Prasad Sahu,
	patches, linux-ide, linux-arm-kernel

This patch enables NCQ support for APM X-Gene SATA controller hardware v1.1
that was broken with hardware v1.0. Second thing, here we should not assume
XGENE_AHCI_V2 always in case of having valid _CID in ACPI table. I need to
remove this assumption because V1_1 also has a valid _CID for backward
compatibly with v1.

v2 changes:
	1. Changed patch description

Signed-off-by: Rameshwar Prasad Sahu <rsahu@apm.com>
---
 drivers/ata/ahci_xgene.c |   14 ++++++++------
 1 files changed, 8 insertions(+), 6 deletions(-)

diff --git a/drivers/ata/ahci_xgene.c b/drivers/ata/ahci_xgene.c
index 73b19b2..8b88be9 100644
--- a/drivers/ata/ahci_xgene.c
+++ b/drivers/ata/ahci_xgene.c
@@ -87,6 +87,7 @@

 enum xgene_ahci_version {
 	XGENE_AHCI_V1 = 1,
+	XGENE_AHCI_V1_1,
 	XGENE_AHCI_V2,
 };

@@ -734,6 +735,7 @@ static struct scsi_host_template ahci_platform_sht = {
 #ifdef CONFIG_ACPI
 static const struct acpi_device_id xgene_ahci_acpi_match[] = {
 	{ "APMC0D0D", XGENE_AHCI_V1},
+	{ "APMC0D67", XGENE_AHCI_V1_1},
 	{ "APMC0D32", XGENE_AHCI_V2},
 	{},
 };
@@ -742,6 +744,7 @@ MODULE_DEVICE_TABLE(acpi, xgene_ahci_acpi_match);

 static const struct of_device_id xgene_ahci_of_match[] = {
 	{.compatible = "apm,xgene-ahci", .data = (void *) XGENE_AHCI_V1},
+	{.compatible = "apm,xgene-ahci-v1-1", .data = (void *) XGENE_AHCI_V1_1},
 	{.compatible = "apm,xgene-ahci-v2", .data = (void *) XGENE_AHCI_V2},
 	{},
 };
@@ -755,8 +758,7 @@ static int xgene_ahci_probe(struct platform_device *pdev)
 	struct resource *res;
 	const struct of_device_id *of_devid;
 	enum xgene_ahci_version version = XGENE_AHCI_V1;
-	const struct ata_port_info *ppi[] = { &xgene_ahci_v1_port_info,
-					      &xgene_ahci_v2_port_info };
+	const struct ata_port_info *ppi;
 	int rc;

 	hpriv = ahci_platform_get_resources(pdev);
@@ -821,8 +823,6 @@ static int xgene_ahci_probe(struct platform_device *pdev)
 				dev_warn(&pdev->dev, "%s: Error reading device info. Assume version1\n",
 					__func__);
 				version = XGENE_AHCI_V1;
-			} else if (info->valid & ACPI_VALID_CID) {
-				version = XGENE_AHCI_V2;
 			}
 		}
 	}
@@ -858,18 +858,20 @@ skip_clk_phy:

 	switch (version) {
 	case XGENE_AHCI_V1:
+		ppi = &xgene_ahci_v1_port_info;
 		hpriv->flags = AHCI_HFLAG_NO_NCQ;
 		break;
 	case XGENE_AHCI_V2:
+		ppi = &xgene_ahci_v2_port_info;
 		hpriv->flags |= AHCI_HFLAG_YES_FBS;
 		hpriv->irq_handler = xgene_ahci_irq_intr;
 		break;
 	default:
+		ppi = &xgene_ahci_v1_port_info;
 		break;
 	}

-	rc = ahci_platform_init_host(pdev, hpriv, ppi[version - 1],
-				     &ahci_platform_sht);
+	rc = ahci_platform_init_host(pdev, hpriv, ppi, &ahci_platform_sht);
 	if (rc)
 		goto disable_resources;

--
1.7.1

^ permalink raw reply related

* [PATCH v5 10/14] sata: ahci-da850: add a workaround for controller instability
From: Bartosz Golaszewski @ 2017-01-20 11:22 UTC (permalink / raw)
  To: Kevin Hilman, Sekhar Nori, Patrick Titiano, Michael Turquette,
	Tejun Heo, Rob Herring, Mark Rutland, Russell King, David Lechner
  Cc: linux-ide, devicetree, linux-kernel, linux-arm-kernel,
	Bartosz Golaszewski
In-Reply-To: <1484911325-23425-1-git-send-email-bgolaszewski@baylibre.com>

We have a use case with the da850 SATA controller where at PLL0
frequency of 456MHz (needed to properly service the LCD controller)
the chip becomes unstable and the hardreset operation is ignored the
first time 50% of times.

The sata core driver already retries to resume the link because some
controllers ignore writes to the SControl register, but just retrying
the resume operation doesn't work - we need to issue he phy/wake reset
again to make it work.

Reimplement ahci_hardreset() in the driver and poke the controller a
couple times before really giving up.

Signed-off-by: Bartosz Golaszewski <bgolaszewski@baylibre.com>
---
 drivers/ata/ahci_da850.c | 28 +++++++++++++++++++++++++++-
 1 file changed, 27 insertions(+), 1 deletion(-)

diff --git a/drivers/ata/ahci_da850.c b/drivers/ata/ahci_da850.c
index 11dd87e..0b2b1a4 100644
--- a/drivers/ata/ahci_da850.c
+++ b/drivers/ata/ahci_da850.c
@@ -16,7 +16,8 @@
 #include <linux/ahci_platform.h>
 #include "ahci.h"
 
-#define DRV_NAME "ahci_da850"
+#define DRV_NAME		"ahci_da850"
+#define HARDRESET_RETRIES	5
 
 /* SATA PHY Control Register offset from AHCI base */
 #define SATA_P0PHYCR_REG	0x178
@@ -76,6 +77,29 @@ static int ahci_da850_softreset(struct ata_link *link,
 	return ret;
 }
 
+static int ahci_da850_hardreset(struct ata_link *link,
+				unsigned int *class, unsigned long deadline)
+{
+	int ret, retry = HARDRESET_RETRIES;
+	bool online;
+
+	/*
+	 * In order to correctly service the LCD controller of the da850 SoC,
+	 * we increased the PLL0 frequency to 456MHz from the default 300MHz.
+	 *
+	 * This made the SATA controller unstable and the hardreset operation
+	 * does not always succeed the first time. Before really giving up to
+	 * bring up the link, retry the reset a couple times.
+	 */
+	do {
+		ret = ahci_do_hardreset(link, class, deadline, &online);
+		if (online)
+			return ret;
+	} while (retry--);
+
+	return ret;
+}
+
 static struct ata_port_operations ahci_da850_port_ops = {
 	.inherits = &ahci_platform_ops,
 	.softreset = ahci_da850_softreset,
@@ -83,6 +107,8 @@ static struct ata_port_operations ahci_da850_port_ops = {
 	 * No need to override .pmp_softreset - it's only used for actual
 	 * PMP-enabled ports.
 	 */
+	.hardreset = ahci_da850_hardreset,
+	.pmp_hardreset = ahci_da850_hardreset,
 };
 
 static const struct ata_port_info ahci_da850_port_info = {
-- 
2.9.3


^ permalink raw reply related

* [PATCH v5 07/14] sata: ahci-da850: add device tree match table
From: Bartosz Golaszewski @ 2017-01-20 11:21 UTC (permalink / raw)
  To: Kevin Hilman, Sekhar Nori, Patrick Titiano, Michael Turquette,
	Tejun Heo, Rob Herring, Mark Rutland, Russell King, David Lechner
  Cc: linux-ide, devicetree, linux-kernel, linux-arm-kernel,
	Bartosz Golaszewski
In-Reply-To: <1484911325-23425-1-git-send-email-bgolaszewski@baylibre.com>

We're using device tree for da850-lcdk. Add the match table to allow
to probe the driver.

Signed-off-by: Bartosz Golaszewski <bgolaszewski@baylibre.com>
---
 drivers/ata/ahci_da850.c | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/drivers/ata/ahci_da850.c b/drivers/ata/ahci_da850.c
index 8cfdc86..7f5328f 100644
--- a/drivers/ata/ahci_da850.c
+++ b/drivers/ata/ahci_da850.c
@@ -121,11 +121,18 @@ static int ahci_da850_probe(struct platform_device *pdev)
 static SIMPLE_DEV_PM_OPS(ahci_da850_pm_ops, ahci_platform_suspend,
 			 ahci_platform_resume);
 
+static const struct of_device_id ahci_da850_of_match[] = {
+	{ .compatible = "ti,da850-ahci", },
+	{ },
+};
+MODULE_DEVICE_TABLE(of, ahci_da850_of_match);
+
 static struct platform_driver ahci_da850_driver = {
 	.probe = ahci_da850_probe,
 	.remove = ata_platform_remove_one,
 	.driver = {
 		.name = DRV_NAME,
+		.of_match_table = ahci_da850_of_match,
 		.pm = &ahci_da850_pm_ops,
 	},
 };
-- 
2.9.3


^ permalink raw reply related

* [PATCH v5 00/14] ARM: da850-lcdk: add SATA support
From: Bartosz Golaszewski @ 2017-01-20 11:21 UTC (permalink / raw)
  To: Kevin Hilman, Sekhar Nori, Patrick Titiano, Michael Turquette,
	Tejun Heo, Rob Herring, Mark Rutland, Russell King, David Lechner
  Cc: linux-ide, devicetree, linux-kernel, linux-arm-kernel,
	Bartosz Golaszewski

This series contains all the changes necessary to make SATA work on
the da850-lcdk board.

The first patch adds DT bindings for the ahci-da850 driver.

The second enables relevant modules in davinci_all_defconfig.

Patches 03/14-06/14 modify the way the clocks are handled regarding
SATA on the da850 platform. We modify the ahci driver to retrieve
the clock via con_id and model the external SATA oscillator as
a real clock.

Patches 07/14-11/14 extend the ahci-da850 driver. Add DT support,
implement workarounds necessary to make SATA work on the da850-lcdk
board and un-hardcode the external clock multiplier.

Patch 12/14 removes a no longer needed BUG_ON.

Last two patches add device tree changes required to probe the
driver.

v1 -> v2:
- dropped patch 04/10 - replaced with local changes in the
  ahci-da850 driver
- added comments explaining the workaround in ahci softreset
- s/0x218000/218000 in the sata DT node label
- added patches chaning the way clocks are handled in the da850 SATA
  code both in arch/ and in the ahci driver
- dropped the clock multiplier property in the DT bindings in favor
  of using struct clk to pass the refclk rate to the driver
- minor tweaks in commit messages

v2 -> v3:
- dropped the clocks property from the ahci-da850 DT binding
- dropped patch 12/14 (SATA pinmux settings)
- dropped an outdated fragment from the commit message in patch 14/14
- s/get_clk()/clk_get()/
- s/connector id/connection id/
- stopped using __div64_32() after noticing that it sometimes produces
  invalid results
- removed the default MPY value from ahci-da850
- registered SATA refclk for board file boot mode as well

v3 -> v4:
- added a patch removing the no longer needed BUG_ON() from
  da850_register_sata()
- fixed indents

v4 ->v5:
- renamed the DT node for the SATA controller from 'ahci' to 'sata',
  while keeping the label as 'sata'
- renamed the SATA node in the DT example as well
- instead of calling the refclk clock 'dummy', called it 'fixed rate'

Bartosz Golaszewski (14):
  devicetree: bindings: add bindings for ahci-da850
  ARM: davinci_all_defconfig: enable SATA modules
  ARM: davinci: add a clock lookup entry for the SATA clock
  sata: ahci-da850: get the sata clock using a connection id
  ARM: davinci: da850: add con_id for the SATA clock
  ARM: davinci: da850: model the SATA refclk
  sata: ahci-da850: add device tree match table
  sata: ahci-da850: implement a workaround for the softreset quirk
  sata: ahci: export ahci_do_hardreset() locally
  sata: ahci-da850: add a workaround for controller instability
  sata: ahci-da850: un-hardcode the MPY bits
  ARM: davinci: remove BUG_ON() from da850_register_sata()
  ARM: dts: da850: add the SATA node
  ARM: dts: da850-lcdk: enable the SATA node

 .../devicetree/bindings/ata/ahci-da850.txt         |  15 ++
 arch/arm/boot/dts/da850-lcdk.dts                   |   4 +
 arch/arm/boot/dts/da850.dtsi                       |   6 +
 arch/arm/configs/davinci_all_defconfig             |   2 +
 arch/arm/mach-davinci/da850.c                      |   2 +-
 arch/arm/mach-davinci/da8xx-dt.c                   |   9 ++
 arch/arm/mach-davinci/devices-da8xx.c              |  30 +++-
 arch/arm/mach-davinci/include/mach/da8xx.h         |   1 +
 drivers/ata/ahci.h                                 |   3 +
 drivers/ata/ahci_da850.c                           | 175 +++++++++++++++++++--
 drivers/ata/libahci.c                              |  18 ++-
 11 files changed, 240 insertions(+), 25 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/ata/ahci-da850.txt

-- 
2.9.3


^ permalink raw reply

* [PATCH v5 14/14] ARM: dts: da850-lcdk: enable the SATA node
From: Bartosz Golaszewski @ 2017-01-20 11:22 UTC (permalink / raw)
  To: Kevin Hilman, Sekhar Nori, Patrick Titiano, Michael Turquette,
	Tejun Heo, Rob Herring, Mark Rutland, Russell King, David Lechner
  Cc: linux-ide-u79uwXL29TY76Z2rM5mHXA,
	devicetree-u79uwXL29TY76Z2rM5mHXA,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA,
	linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
	Bartosz Golaszewski
In-Reply-To: <1484911325-23425-1-git-send-email-bgolaszewski-rdvid1DuHRBWk0Htik3J/w@public.gmane.org>

Enable the SATA node for da850-lcdk. We omit the pinctrl property on
purpose - the muxed SATA pins are not hooked up to anything
SATA-related on the lcdk.

Signed-off-by: Bartosz Golaszewski <bgolaszewski-rdvid1DuHRBWk0Htik3J/w@public.gmane.org>
---
 arch/arm/boot/dts/da850-lcdk.dts | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/arch/arm/boot/dts/da850-lcdk.dts b/arch/arm/boot/dts/da850-lcdk.dts
index afcb482..fbeee3c 100644
--- a/arch/arm/boot/dts/da850-lcdk.dts
+++ b/arch/arm/boot/dts/da850-lcdk.dts
@@ -105,6 +105,10 @@
 	status = "okay";
 };
 
+&sata {
+	status = "okay";
+};
+
 &mdio {
 	pinctrl-names = "default";
 	pinctrl-0 = <&mdio_pins>;
-- 
2.9.3

--
To unsubscribe from this list: send the line "unsubscribe devicetree" 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 related

* [PATCH v5 13/14] ARM: dts: da850: add the SATA node
From: Bartosz Golaszewski @ 2017-01-20 11:22 UTC (permalink / raw)
  To: Kevin Hilman, Sekhar Nori, Patrick Titiano, Michael Turquette,
	Tejun Heo, Rob Herring, Mark Rutland, Russell King, David Lechner
  Cc: linux-ide, devicetree, linux-kernel, linux-arm-kernel,
	Bartosz Golaszewski
In-Reply-To: <1484911325-23425-1-git-send-email-bgolaszewski@baylibre.com>

Add the SATA node to the da850 device tree.

Signed-off-by: Bartosz Golaszewski <bgolaszewski@baylibre.com>
---
 arch/arm/boot/dts/da850.dtsi | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/arch/arm/boot/dts/da850.dtsi b/arch/arm/boot/dts/da850.dtsi
index 104155d..3b5fd41e 100644
--- a/arch/arm/boot/dts/da850.dtsi
+++ b/arch/arm/boot/dts/da850.dtsi
@@ -403,6 +403,12 @@
 			phy-names = "usb-phy";
 			status = "disabled";
 		};
+		sata: sata@218000 {
+			compatible = "ti,da850-ahci";
+			reg = <0x218000 0x2000>, <0x22c018 0x4>;
+			interrupts = <67>;
+			status = "disabled";
+		};
 		mdio: mdio@224000 {
 			compatible = "ti,davinci_mdio";
 			#address-cells = <1>;
-- 
2.9.3

^ permalink raw reply related

* [PATCH v5 12/14] ARM: davinci: remove BUG_ON() from da850_register_sata()
From: Bartosz Golaszewski @ 2017-01-20 11:22 UTC (permalink / raw)
  To: Kevin Hilman, Sekhar Nori, Patrick Titiano, Michael Turquette,
	Tejun Heo, Rob Herring, Mark Rutland, Russell King, David Lechner
  Cc: linux-ide, devicetree, linux-kernel, linux-arm-kernel,
	Bartosz Golaszewski
In-Reply-To: <1484911325-23425-1-git-send-email-bgolaszewski@baylibre.com>

The ahci driver now supports other refclk clock rates.

Signed-off-by: Bartosz Golaszewski <bgolaszewski@baylibre.com>
---
 arch/arm/mach-davinci/devices-da8xx.c | 3 ---
 1 file changed, 3 deletions(-)

diff --git a/arch/arm/mach-davinci/devices-da8xx.c b/arch/arm/mach-davinci/devices-da8xx.c
index cfceb32..7cf529f 100644
--- a/arch/arm/mach-davinci/devices-da8xx.c
+++ b/arch/arm/mach-davinci/devices-da8xx.c
@@ -1080,9 +1080,6 @@ int __init da850_register_sata(unsigned long refclkpn)
 {
 	int ret;
 
-	/* please see comment in drivers/ata/ahci_da850.c */
-	BUG_ON(refclkpn != 100 * 1000 * 1000);
-
 	ret = da850_register_sata_refclk(refclkpn);
 	if (ret)
 		return ret;
-- 
2.9.3

^ permalink raw reply related

* [PATCH v5 11/14] sata: ahci-da850: un-hardcode the MPY bits
From: Bartosz Golaszewski @ 2017-01-20 11:22 UTC (permalink / raw)
  To: Kevin Hilman, Sekhar Nori, Patrick Titiano, Michael Turquette,
	Tejun Heo, Rob Herring, Mark Rutland, Russell King, David Lechner
  Cc: linux-ide, devicetree, linux-kernel, linux-arm-kernel,
	Bartosz Golaszewski
In-Reply-To: <1484911325-23425-1-git-send-email-bgolaszewski@baylibre.com>

All platforms using this driver now register the SATA refclk. Remove
the hardcoded default value from the driver and instead read the rate
of the external clock and calculate the required MPY value from it.

Signed-off-by: Bartosz Golaszewski <bgolaszewski@baylibre.com>
---
 drivers/ata/ahci_da850.c | 91 ++++++++++++++++++++++++++++++++++++++++--------
 1 file changed, 76 insertions(+), 15 deletions(-)

diff --git a/drivers/ata/ahci_da850.c b/drivers/ata/ahci_da850.c
index 0b2b1a4..9ed404d 100644
--- a/drivers/ata/ahci_da850.c
+++ b/drivers/ata/ahci_da850.c
@@ -29,17 +29,8 @@
 #define SATA_PHY_TXSWING(x)	((x) << 19)
 #define SATA_PHY_ENPLL(x)	((x) << 31)
 
-/*
- * The multiplier needed for 1.5GHz PLL output.
- *
- * NOTE: This is currently hardcoded to be suitable for 100MHz crystal
- * frequency (which is used by DA850 EVM board) and may need to be changed
- * if you would like to use this driver on some other board.
- */
-#define DA850_SATA_CLK_MULTIPLIER	7
-
 static void da850_sata_init(struct device *dev, void __iomem *pwrdn_reg,
-			    void __iomem *ahci_base)
+			    void __iomem *ahci_base, u32 mpy)
 {
 	unsigned int val;
 
@@ -48,13 +39,61 @@ static void da850_sata_init(struct device *dev, void __iomem *pwrdn_reg,
 	val &= ~BIT(0);
 	writel(val, pwrdn_reg);
 
-	val = SATA_PHY_MPY(DA850_SATA_CLK_MULTIPLIER + 1) | SATA_PHY_LOS(1) |
-	      SATA_PHY_RXCDR(4) | SATA_PHY_RXEQ(1) | SATA_PHY_TXSWING(3) |
-	      SATA_PHY_ENPLL(1);
+	val = SATA_PHY_MPY(mpy) | SATA_PHY_LOS(1) | SATA_PHY_RXCDR(4) |
+	      SATA_PHY_RXEQ(1) | SATA_PHY_TXSWING(3) | SATA_PHY_ENPLL(1);
 
 	writel(val, ahci_base + SATA_P0PHYCR_REG);
 }
 
+static u32 ahci_da850_calculate_mpy(unsigned long refclk_rate)
+{
+	u32 pll_output = 1500000000, needed;
+
+	/*
+	 * We need to determine the value of the multiplier (MPY) bits.
+	 * In order to include the 12.5 multiplier we need to first divide
+	 * the refclk rate by ten.
+	 *
+	 * __div64_32() turned out to be unreliable, sometimes returning
+	 * false results.
+	 */
+	WARN((refclk_rate % 10) != 0, "refclk must be divisible by 10");
+	needed = pll_output / (refclk_rate / 10);
+
+	/*
+	 * What we have now is (multiplier * 10).
+	 *
+	 * Let's determine the actual register value we need to write.
+	 */
+
+	switch (needed) {
+	case 50:
+		return 0x1;
+	case 60:
+		return 0x2;
+	case 80:
+		return 0x4;
+	case 100:
+		return 0x5;
+	case 120:
+		return 0x6;
+	case 125:
+		return 0x7;
+	case 150:
+		return 0x8;
+	case 200:
+		return 0x9;
+	case 250:
+		return 0xa;
+	default:
+		/*
+		 * We should have divided evenly - if not, return an invalid
+		 * value.
+		 */
+		return 0;
+	}
+}
+
 static int ahci_da850_softreset(struct ata_link *link,
 				unsigned int *class, unsigned long deadline)
 {
@@ -126,9 +165,10 @@ static int ahci_da850_probe(struct platform_device *pdev)
 {
 	struct device *dev = &pdev->dev;
 	struct ahci_host_priv *hpriv;
-	struct resource *res;
 	void __iomem *pwrdn_reg;
+	struct resource *res;
 	struct clk *clk;
+	u32 mpy;
 	int rc;
 
 	hpriv = ahci_platform_get_resources(pdev);
@@ -150,6 +190,27 @@ static int ahci_da850_probe(struct platform_device *pdev)
 		hpriv->clks[0] = clk;
 	}
 
+	/*
+	 * The second clock used by ahci-da850 is the external REFCLK. If we
+	 * didn't get it from ahci_platform_get_resources(), let's try to
+	 * specify the con_id in clk_get().
+	 */
+	if (!hpriv->clks[1]) {
+		clk = clk_get(dev, "refclk");
+		if (IS_ERR(clk)) {
+			dev_err(dev, "unable to obtain the reference clock");
+			return -ENODEV;
+		} else {
+			hpriv->clks[1] = clk;
+		}
+	}
+
+	mpy = ahci_da850_calculate_mpy(clk_get_rate(hpriv->clks[1]));
+	if (mpy == 0) {
+		dev_err(dev, "invalid REFCLK multiplier value: 0x%x", mpy);
+		return -EINVAL;
+	}
+
 	rc = ahci_platform_enable_resources(hpriv);
 	if (rc)
 		return rc;
@@ -162,7 +223,7 @@ static int ahci_da850_probe(struct platform_device *pdev)
 	if (!pwrdn_reg)
 		goto disable_resources;
 
-	da850_sata_init(dev, pwrdn_reg, hpriv->mmio);
+	da850_sata_init(dev, pwrdn_reg, hpriv->mmio, mpy);
 
 	rc = ahci_platform_init_host(pdev, hpriv, &ahci_da850_port_info,
 				     &ahci_platform_sht);
-- 
2.9.3

^ permalink raw reply related

* [PATCH v5 09/14] sata: ahci: export ahci_do_hardreset() locally
From: Bartosz Golaszewski @ 2017-01-20 11:22 UTC (permalink / raw)
  To: Kevin Hilman, Sekhar Nori, Patrick Titiano, Michael Turquette,
	Tejun Heo, Rob Herring, Mark Rutland, Russell King, David Lechner
  Cc: linux-ide-u79uwXL29TY76Z2rM5mHXA,
	devicetree-u79uwXL29TY76Z2rM5mHXA,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA,
	linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
	Bartosz Golaszewski
In-Reply-To: <1484911325-23425-1-git-send-email-bgolaszewski-rdvid1DuHRBWk0Htik3J/w@public.gmane.org>

We need a way to retrieve the information about the online state of
the link in the ahci-da850 driver.

Create a new function: ahci_do_hardreset() which is called from
ahci_hardreset() for backwards compatibility, but has an additional
argument: 'online' - which can be used to check if the link is online
after this function returns.

The new routine will be used in the ahci-da850 driver to avoid code
duplication when implementing a workaround for tha da850 SATA
controller quirk/instability.

Signed-off-by: Bartosz Golaszewski <bgolaszewski-rdvid1DuHRBWk0Htik3J/w@public.gmane.org>
---
 drivers/ata/ahci.h    |  3 +++
 drivers/ata/libahci.c | 18 +++++++++++++-----
 2 files changed, 16 insertions(+), 5 deletions(-)

diff --git a/drivers/ata/ahci.h b/drivers/ata/ahci.h
index 0cc08f8..5db6ab2 100644
--- a/drivers/ata/ahci.h
+++ b/drivers/ata/ahci.h
@@ -398,6 +398,9 @@ int ahci_do_softreset(struct ata_link *link, unsigned int *class,
 		      int pmp, unsigned long deadline,
 		      int (*check_ready)(struct ata_link *link));
 
+int ahci_do_hardreset(struct ata_link *link, unsigned int *class,
+		      unsigned long deadline, bool *online);
+
 unsigned int ahci_qc_issue(struct ata_queued_cmd *qc);
 int ahci_stop_engine(struct ata_port *ap);
 void ahci_start_fis_rx(struct ata_port *ap);
diff --git a/drivers/ata/libahci.c b/drivers/ata/libahci.c
index ee7db31..3159f9e 100644
--- a/drivers/ata/libahci.c
+++ b/drivers/ata/libahci.c
@@ -1519,8 +1519,8 @@ static int ahci_pmp_retry_softreset(struct ata_link *link, unsigned int *class,
 	return rc;
 }
 
-static int ahci_hardreset(struct ata_link *link, unsigned int *class,
-			  unsigned long deadline)
+int ahci_do_hardreset(struct ata_link *link, unsigned int *class,
+		      unsigned long deadline, bool *online)
 {
 	const unsigned long *timing = sata_ehc_deb_timing(&link->eh_context);
 	struct ata_port *ap = link->ap;
@@ -1528,7 +1528,6 @@ static int ahci_hardreset(struct ata_link *link, unsigned int *class,
 	struct ahci_host_priv *hpriv = ap->host->private_data;
 	u8 *d2h_fis = pp->rx_fis + RX_FIS_D2H_REG;
 	struct ata_taskfile tf;
-	bool online;
 	int rc;
 
 	DPRINTK("ENTER\n");
@@ -1540,17 +1539,26 @@ static int ahci_hardreset(struct ata_link *link, unsigned int *class,
 	tf.command = ATA_BUSY;
 	ata_tf_to_fis(&tf, 0, 0, d2h_fis);
 
-	rc = sata_link_hardreset(link, timing, deadline, &online,
+	rc = sata_link_hardreset(link, timing, deadline, online,
 				 ahci_check_ready);
 
 	hpriv->start_engine(ap);
 
-	if (online)
+	if (*online)
 		*class = ahci_dev_classify(ap);
 
 	DPRINTK("EXIT, rc=%d, class=%u\n", rc, *class);
 	return rc;
 }
+EXPORT_SYMBOL_GPL(ahci_do_hardreset);
+
+static int ahci_hardreset(struct ata_link *link, unsigned int *class,
+			  unsigned long deadline)
+{
+	bool online;
+
+	return ahci_do_hardreset(link, class, deadline, &online);
+}
 
 static void ahci_postreset(struct ata_link *link, unsigned int *class)
 {
-- 
2.9.3

--
To unsubscribe from this list: send the line "unsubscribe devicetree" 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 related

* [PATCH v5 08/14] sata: ahci-da850: implement a workaround for the softreset quirk
From: Bartosz Golaszewski @ 2017-01-20 11:21 UTC (permalink / raw)
  To: Kevin Hilman, Sekhar Nori, Patrick Titiano, Michael Turquette,
	Tejun Heo, Rob Herring, Mark Rutland, Russell King, David Lechner
  Cc: linux-ide, devicetree, linux-kernel, linux-arm-kernel,
	Bartosz Golaszewski
In-Reply-To: <1484911325-23425-1-git-send-email-bgolaszewski@baylibre.com>

There's an issue with the da850 SATA controller: if port multiplier
support is compiled in, but we're connecting the drive directly to
the SATA port on the board, the drive can't be detected.

To make SATA work on the da850-lcdk board: first try to softreset
with pmp - if the operation fails with -EBUSY, retry without pmp.

Signed-off-by: Bartosz Golaszewski <bgolaszewski@baylibre.com>
---
 drivers/ata/ahci_da850.c | 33 ++++++++++++++++++++++++++++++++-
 1 file changed, 32 insertions(+), 1 deletion(-)

diff --git a/drivers/ata/ahci_da850.c b/drivers/ata/ahci_da850.c
index 7f5328f..11dd87e 100644
--- a/drivers/ata/ahci_da850.c
+++ b/drivers/ata/ahci_da850.c
@@ -54,11 +54,42 @@ static void da850_sata_init(struct device *dev, void __iomem *pwrdn_reg,
 	writel(val, ahci_base + SATA_P0PHYCR_REG);
 }
 
+static int ahci_da850_softreset(struct ata_link *link,
+				unsigned int *class, unsigned long deadline)
+{
+	int pmp, ret;
+
+	pmp = sata_srst_pmp(link);
+
+	/*
+	 * There's an issue with the SATA controller on da850 SoCs: if we
+	 * enable Port Multiplier support, but the drive is connected directly
+	 * to the board, it can't be detected. As a workaround: if PMP is
+	 * enabled, we first call ahci_do_softreset() and pass it the result of
+	 * sata_srst_pmp(). If this call fails, we retry with pmp = 0.
+	 */
+	ret = ahci_do_softreset(link, class, pmp, deadline, ahci_check_ready);
+	if (pmp && ret == -EBUSY)
+		return ahci_do_softreset(link, class, 0,
+					 deadline, ahci_check_ready);
+
+	return ret;
+}
+
+static struct ata_port_operations ahci_da850_port_ops = {
+	.inherits = &ahci_platform_ops,
+	.softreset = ahci_da850_softreset,
+	/*
+	 * No need to override .pmp_softreset - it's only used for actual
+	 * PMP-enabled ports.
+	 */
+};
+
 static const struct ata_port_info ahci_da850_port_info = {
 	.flags		= AHCI_FLAG_COMMON,
 	.pio_mask	= ATA_PIO4,
 	.udma_mask	= ATA_UDMA6,
-	.port_ops	= &ahci_platform_ops,
+	.port_ops	= &ahci_da850_port_ops,
 };
 
 static struct scsi_host_template ahci_platform_sht = {
-- 
2.9.3

^ permalink raw reply related

* [PATCH v5 06/14] ARM: davinci: da850: model the SATA refclk
From: Bartosz Golaszewski @ 2017-01-20 11:21 UTC (permalink / raw)
  To: Kevin Hilman, Sekhar Nori, Patrick Titiano, Michael Turquette,
	Tejun Heo, Rob Herring, Mark Rutland, Russell King, David Lechner
  Cc: linux-ide, devicetree, linux-kernel, linux-arm-kernel,
	Bartosz Golaszewski
In-Reply-To: <1484911325-23425-1-git-send-email-bgolaszewski@baylibre.com>

Register a fixed rate clock modelling the external SATA oscillator
for da850 (both DT and board file mode).

Signed-off-by: Bartosz Golaszewski <bgolaszewski@baylibre.com>
---
 arch/arm/mach-davinci/da8xx-dt.c           |  8 ++++++++
 arch/arm/mach-davinci/devices-da8xx.c      | 29 +++++++++++++++++++++++++++++
 arch/arm/mach-davinci/include/mach/da8xx.h |  1 +
 3 files changed, 38 insertions(+)

diff --git a/arch/arm/mach-davinci/da8xx-dt.c b/arch/arm/mach-davinci/da8xx-dt.c
index b83e5d1..55342ca 100644
--- a/arch/arm/mach-davinci/da8xx-dt.c
+++ b/arch/arm/mach-davinci/da8xx-dt.c
@@ -50,6 +50,9 @@ static struct of_dev_auxdata da850_auxdata_lookup[] __initdata = {
 
 static void __init da850_init_machine(void)
 {
+	/* All existing boards use 100MHz SATA refclkpn */
+	static const unsigned long sata_refclkpn = 100 * 1000 * 1000;
+
 	int ret;
 
 	ret = da8xx_register_usb20_phy_clk(false);
@@ -61,6 +64,11 @@ static void __init da850_init_machine(void)
 		pr_warn("%s: registering USB 1.1 PHY clock failed: %d",
 			__func__, ret);
 
+	ret = da850_register_sata_refclk(sata_refclkpn);
+	if (ret)
+		pr_warn("%s: registering SATA REFCLK failed: %d",
+			__func__, ret);
+
 	of_platform_default_populate(NULL, da850_auxdata_lookup, NULL);
 	davinci_pm_init();
 }
diff --git a/arch/arm/mach-davinci/devices-da8xx.c b/arch/arm/mach-davinci/devices-da8xx.c
index c2457b3..cfceb32 100644
--- a/arch/arm/mach-davinci/devices-da8xx.c
+++ b/arch/arm/mach-davinci/devices-da8xx.c
@@ -24,6 +24,7 @@
 #include <mach/common.h>
 #include <mach/time.h>
 #include <mach/da8xx.h>
+#include <mach/clock.h>
 #include "cpuidle.h"
 #include "sram.h"
 
@@ -1023,6 +1024,28 @@ int __init da8xx_register_spi_bus(int instance, unsigned num_chipselect)
 }
 
 #ifdef CONFIG_ARCH_DAVINCI_DA850
+static struct clk sata_refclk = {
+	.name		= "sata_refclk",
+	.set_rate	= davinci_simple_set_rate,
+};
+
+static struct clk_lookup sata_refclk_lookup =
+		CLK("ahci_da850", "refclk", &sata_refclk);
+
+int __init da850_register_sata_refclk(int rate)
+{
+	int ret;
+
+	sata_refclk.rate = rate;
+	ret = clk_register(&sata_refclk);
+	if (ret)
+		return ret;
+
+	clkdev_add(&sata_refclk_lookup);
+
+	return 0;
+}
+
 static struct resource da850_sata_resources[] = {
 	{
 		.start	= DA850_SATA_BASE,
@@ -1055,9 +1078,15 @@ static struct platform_device da850_sata_device = {
 
 int __init da850_register_sata(unsigned long refclkpn)
 {
+	int ret;
+
 	/* please see comment in drivers/ata/ahci_da850.c */
 	BUG_ON(refclkpn != 100 * 1000 * 1000);
 
+	ret = da850_register_sata_refclk(refclkpn);
+	if (ret)
+		return ret;
+
 	return platform_device_register(&da850_sata_device);
 }
 #endif
diff --git a/arch/arm/mach-davinci/include/mach/da8xx.h b/arch/arm/mach-davinci/include/mach/da8xx.h
index 85ff218..7e46422 100644
--- a/arch/arm/mach-davinci/include/mach/da8xx.h
+++ b/arch/arm/mach-davinci/include/mach/da8xx.h
@@ -95,6 +95,7 @@ int da8xx_register_usb11(struct da8xx_ohci_root_hub *pdata);
 int da8xx_register_usb_refclkin(int rate);
 int da8xx_register_usb20_phy_clk(bool use_usb_refclkin);
 int da8xx_register_usb11_phy_clk(bool use_usb_refclkin);
+int da850_register_sata_refclk(int rate);
 int da8xx_register_emac(void);
 int da8xx_register_uio_pruss(void);
 int da8xx_register_lcdc(struct da8xx_lcdc_platform_data *pdata);
-- 
2.9.3

^ permalink raw reply related

* [PATCH v5 05/14] ARM: davinci: da850: add con_id for the SATA clock
From: Bartosz Golaszewski @ 2017-01-20 11:21 UTC (permalink / raw)
  To: Kevin Hilman, Sekhar Nori, Patrick Titiano, Michael Turquette,
	Tejun Heo, Rob Herring, Mark Rutland, Russell King, David Lechner
  Cc: linux-ide, Bartosz Golaszewski, linux-kernel, linux-arm-kernel,
	devicetree
In-Reply-To: <1484911325-23425-1-git-send-email-bgolaszewski@baylibre.com>

The ahci-da850 SATA driver is now capable of retrieving clocks by
con_id. Add the connection id for the sysclk2-derived SATA clock.

Signed-off-by: Bartosz Golaszewski <bgolaszewski@baylibre.com>
---
 arch/arm/mach-davinci/da850.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/arm/mach-davinci/da850.c b/arch/arm/mach-davinci/da850.c
index 1d873d1..dbf1daa 100644
--- a/arch/arm/mach-davinci/da850.c
+++ b/arch/arm/mach-davinci/da850.c
@@ -571,7 +571,7 @@ static struct clk_lookup da850_clks[] = {
 	CLK("spi_davinci.0",	NULL,		&spi0_clk),
 	CLK("spi_davinci.1",	NULL,		&spi1_clk),
 	CLK("vpif",		NULL,		&vpif_clk),
-	CLK("ahci_da850",		NULL,		&sata_clk),
+	CLK("ahci_da850",	"sata",		&sata_clk),
 	CLK("davinci-rproc.0",	NULL,		&dsp_clk),
 	CLK(NULL,		NULL,		&ehrpwm_clk),
 	CLK("ehrpwm.0",		"fck",		&ehrpwm0_clk),
-- 
2.9.3

^ permalink raw reply related


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