* Re: [PATCH v6 05/14] ARM: davinci: da850: add con_id for the SATA clock
From: Sekhar Nori @ 2017-01-26 17:40 UTC (permalink / raw)
To: Grygorii Strashko, Bartosz Golaszewski, Kevin Hilman,
Patrick Titiano, Michael Turquette, Tejun Heo, Rob Herring,
Mark Rutland, Russell King, David Lechner
Cc: linux-ide, linux-kernel, linux-arm-kernel, devicetree
In-Reply-To: <93a3bbb3-2d9f-bc38-c7b4-5eb3325d985d@ti.com>
On Thursday 26 January 2017 10:26 PM, Grygorii Strashko wrote:
>
>
> On 01/23/2017 11:00 AM, Bartosz Golaszewski wrote:
>> 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),
>
> I'm worry a bit - wouldn't this cause future problems with PM runtime
> (if it will be the case)?
>
> If this is functional clock - shouldn't it be "fck" to
> follow PM domain con_id list for davinci? (arch/arm/mach-davinci/pm_domain.c)
I agree with Grygorii. Calling this clock "fck" will make it easy to
convert the DA850 AHCI driver to use pm_runtime at a future date (no
mach-davinci changes should be needed).
Sorry about not spotting this earlier.
Thanks,
Sekhar
^ permalink raw reply
* Re: [PATCH v6 05/14] ARM: davinci: da850: add con_id for the SATA clock
From: Grygorii Strashko @ 2017-01-26 16:56 UTC (permalink / raw)
To: Bartosz Golaszewski, Kevin Hilman, Sekhar Nori, Patrick Titiano,
Michael Turquette, Tejun Heo, Rob Herring, Mark Rutland,
Russell King, David Lechner
Cc: linux-ide, linux-kernel, linux-arm-kernel, devicetree
In-Reply-To: <1485190856-4711-6-git-send-email-bgolaszewski@baylibre.com>
On 01/23/2017 11:00 AM, Bartosz Golaszewski wrote:
> 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),
I'm worry a bit - wouldn't this cause future problems with PM runtime
(if it will be the case)?
If this is functional clock - shouldn't it be "fck" to
follow PM domain con_id list for davinci? (arch/arm/mach-davinci/pm_domain.c)
> CLK("davinci-rproc.0", NULL, &dsp_clk),
> CLK(NULL, NULL, &ehrpwm_clk),
> CLK("ehrpwm.0", "fck", &ehrpwm0_clk),
>
--
regards,
-grygorii
^ permalink raw reply
* Re: [PATCH 0/3] ata: add m68k/Atari Falcon PATA support
From: Geert Uytterhoeven @ 2017-01-26 9:03 UTC (permalink / raw)
To: Finn Thain
Cc: Michael Schmitz, Bartlomiej Zolnierkiewicz, Tejun Heo,
linux-ide@vger.kernel.org, Linux/m68k, Linux Kernel Development,
Andreas Schwab
In-Reply-To: <alpine.LNX.2.00.1701261944160.23370@nippy.intranet>
Hi Finn,
On Thu, Jan 26, 2017 at 9:47 AM, Finn Thain <fthain@telegraphics.com.au> wrote:
> The difficulty will be arranging for disabled FDC & IDE interrupt sources
> during SCSI DMA, and disabled SCSI & IDE interrupt sources during FDC DMA.
> (Not all 5380 interrupts can be disabled; no idea about the IDE device or
> WD1772 FDC.)
IDE interrupts are disabled at the device level.
Unfortunately some hard drives (e.g. Western Digital Caviar) didn't honour
the ATA disable IRQ bit, so they caused an interrupt deadlock if you probed
for them on Amiga with the IDE interrupt enabled. The problem didn't show
up on PC because they had no shared interrupts, while on A4000 the IDE
interrupt is shared with Zorro Ethernet, which was still enabled.
That was fixed (in 1995 or 1996?) by disabling the IDE interrupt at the
IRQ controller level.
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
^ permalink raw reply
* Re: [PATCH 0/3] ata: add m68k/Atari Falcon PATA support
From: Finn Thain @ 2017-01-26 8:47 UTC (permalink / raw)
To: Michael Schmitz
Cc: Bartlomiej Zolnierkiewicz, Tejun Heo, Geert Uytterhoeven,
linux-ide, Linux/m68k, Linux Kernel Development, Andreas Schwab
In-Reply-To: <1585626b-5642-3e42-2d40-52fae49c9e59@gmail.com>
On Mon, 23 Jan 2017, Michael Schmitz wrote:
>
> Am 21.01.2017 um 20:37 schrieb Finn Thain:
>
> >
> > Actually, the fundamental problem you are describing is partly solved.
> > By polling for DMA completion with local irqs disabled, we mostly
> > avoid the need for the stdma.c "lock" because FDC/SCSI/IDE interrupt
> > handlers can never interfere with a FDC/SCSI DMA process that might be
> > underway.
>
> I hadn't considered that. Can PDMA for Falcon SCSI coexist with
> interrupt-using DMA for TT SCSI in the same driver (i.e. as runtime
> options)?
Sure, why not?
> How much overhead and latency would polling for DMA completion add?
>
A polled DMA transfer should be faster than PDMA (i.e. mac_scsi, g_NCR5380
etc). mac_scsi gets about 0.5 MBps from PDMA with sg_tablesize == 1, and I
hope that DMA could get twice that (notwithstanding dumb hardware design).
This would imply CPU overhead that is half of that which mac_scsi incurs.
That's the best case, but I see no reason to expect worse performance than
PDMA gets.
> atari_irq_pending(IRQ_MFP_FSCSI) should show the interrupt pending
> condition if you want to poll for it.
The difficulty will be arranging for disabled FDC & IDE interrupt sources
during SCSI DMA, and disabled SCSI & IDE interrupt sources during FDC DMA.
(Not all 5380 interrupts can be disabled; no idea about the IDE device or
WD1772 FDC.)
But if that is impossible, we just have to detect the short DMA that might
result from an undesired interrupt.
> That's actually given me another idea to pursue - if we can ensure the
> IDE interrupt handler is always run first,
There are no interrupts from the ATA driver you're testing, right? If you
would re-introduce them, the whole polled DMA idea is moot.
> and check whether the interrupt is still pending when the SCSI or floppy
> interrupt handler runs and DMA has been in progress, we should be able
> to avoid calling the respective handlers unnecessarily.
>
> (The output of atari_irq_pending() does not directly reflect the status
> of the MFP IRQ inputs - that would require testing bits in
> st_mfp.par_dt_reg instead. )
>
> > I don't think the IDE/ATA driver needs to be included. atari_scsi and
> > ataflop would though (if both drivers need DMA transfers).
>
> If we manage to separate interrupt sharing from DMA access locking, IDE
> would not need to take part in the locking. I'm assuming that IDE can
> cope with spurious interrupts and won't get confused by a SCSI
> interrupt.
>
The ATA driver will never have to cope with a spurious interrupt under my
simplifying assumptions discussed earlier, so the spurious interrupt
question seems to belong to some alternative approach...
> I think it could work both ways - polling for DMA completion or avoiding
> to call the SCSI interrupt handler the interrupt was caused by IDE only.
> But it's indeed time to put that to the test.
>
... "Both ways"? I don't follow. I don't see how IDE can share the FDC and
SCSI interrupt line without sharing the stdma.c locking scheme. What is
the alternative approach (i.e not polled DMA) that you alude to?
--
^ permalink raw reply
* Re: [PATCH] pata_legacy: Allow disabling of legacy PATA device probes on non-PCI systems
From: Tejun Heo @ 2017-01-24 16:19 UTC (permalink / raw)
To: whiteheadm
Cc: Gwendal Grignou, One Thousand Gnomes, Greg Kroah-Hartman,
Sergei Shtylyov, IDE/ATA development list
In-Reply-To: <CAP8WD_aya3junhovXtc=iUaaX43=Y1s8-eya_LvN_BQjXGLbew@mail.gmail.com>
Hello, Matthew.
On Mon, Jan 23, 2017 at 06:57:34PM -0500, tedheadster wrote:
> Gwendal,
>
> >> 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.
> > That's the root cause of pata port not released. Can we run your debug
> > code one more time, enabling ap->tdev.kobj.release_debug?
> >
>
> I'm building the debug kernel because I have the test hardware, but
> I'm not quite sure what I need to do to enable
> ap->tdev.kobj.release_debug. Tejun's patch enables kobj.release_debug,
> but I'm not sure if that is the same thing you want. Please advise.
The following patch should printout the same debug info for all
transport kobjects.
Thanks!
diff --git a/drivers/ata/libata-transport.c b/drivers/ata/libata-transport.c
index 7ef16c0..80d72a6 100644
--- a/drivers/ata/libata-transport.c
+++ b/drivers/ata/libata-transport.c
@@ -283,6 +283,7 @@ int ata_tport_add(struct device *parent,
device_initialize(dev);
dev->type = &ata_port_type;
+ dev->kobj.release_debug = true;
dev->parent = get_device(parent);
dev->release = ata_tport_release;
@@ -412,6 +413,7 @@ int ata_tlink_add(struct ata_link *link)
device_initialize(dev);
dev->parent = get_device(&ap->tdev);
dev->release = ata_tlink_release;
+ dev->kobj.release_debug = true;
if (ata_is_host_link(link))
dev_set_name(dev, "link%d", ap->print_id);
else
@@ -664,6 +666,7 @@ static int ata_tdev_add(struct ata_device *ata_dev)
device_initialize(dev);
dev->parent = get_device(&link->tdev);
dev->release = ata_tdev_release;
+ dev->kobj.release_debug = true;
if (ata_is_host_link(link))
dev_set_name(dev, "dev%d.%d", ap->print_id,ata_dev->devno);
else
diff --git a/drivers/ata/pata_legacy.c b/drivers/ata/pata_legacy.c
index 53828b6c..074bb45 100644
--- a/drivers/ata/pata_legacy.c
+++ b/drivers/ata/pata_legacy.c
@@ -965,6 +965,8 @@ static __init int legacy_init_one(struct legacy_probe *probe)
if (IS_ERR(pdev))
return PTR_ERR(pdev);
+ pdev->dev.kobj.release_debug = true;
+
ret = -EBUSY;
if (devm_request_region(&pdev->dev, io, 8, "pata_legacy") == NULL ||
devm_request_region(&pdev->dev, io + 0x0206, 1,
diff --git a/drivers/base/core.c b/drivers/base/core.c
index 020ea7f..6666a49 100644
--- a/drivers/base/core.c
+++ b/drivers/base/core.c
@@ -803,8 +803,17 @@ static void device_release(struct kobject *kobj)
* is deleted but alive, so release devres here to avoid
* possible memory leak.
*/
+ if (kobj->release_debug)
+ dev_info(dev, "XXX device_release: drel=%pf trel=%pf cdrel=%pf\n",
+ dev->release,
+ dev->type ? dev->type->release : NULL,
+ dev->class ? dev->class->dev_release : NULL);
+
devres_release_all(dev);
+ if (kobj->release_debug)
+ dev_info(dev, "XXX device_release: devres_release_all() done\n");
+
if (dev->release)
dev->release(dev);
else if (dev->type && dev->type->release)
diff --git a/include/linux/kobject.h b/include/linux/kobject.h
index e628459..e1740de 100644
--- a/include/linux/kobject.h
+++ b/include/linux/kobject.h
@@ -76,6 +76,7 @@ struct kobject {
unsigned int state_add_uevent_sent:1;
unsigned int state_remove_uevent_sent:1;
unsigned int uevent_suppress:1;
+ unsigned int release_debug:1;
};
extern __printf(2, 3)
diff --git a/lib/kobject.c b/lib/kobject.c
index 445dcae..058c486 100644
--- a/lib/kobject.c
+++ b/lib/kobject.c
@@ -595,6 +595,13 @@ struct kobject *kobject_get(struct kobject *kobj)
WARN(1, KERN_WARNING "kobject: '%s' (%p): is not "
"initialized, yet kobject_get() is being "
"called.\n", kobject_name(kobj), kobj);
+ if (kobj->release_debug)
+ printk("XXX kobject_get(%s): ref=%d++ (%pf:%pf:%pf)\n",
+ kobject_name(kobj),
+ atomic_read(&kobj->kref.refcount),
+ __builtin_return_address(1),
+ __builtin_return_address(2),
+ __builtin_return_address(3));
kref_get(&kobj->kref);
}
return kobj;
@@ -620,6 +627,14 @@ static void kobject_cleanup(struct kobject *kobj)
pr_debug("kobject: '%s' (%p): %s, parent %p\n",
kobject_name(kobj), kobj, __func__, kobj->parent);
+ if (kobj->release_debug)
+ printk("XXX kobject_cleanup(%s): rel=%pf (%pf:%pf:%pf)\n",
+ kobject_name(kobj),
+ (t && t->release) ? t->release : NULL,
+ __builtin_return_address(1),
+ __builtin_return_address(2),
+ __builtin_return_address(3));
+
if (t && !t->release)
pr_debug("kobject: '%s' (%p): does not have a release() "
"function, it is broken and must be fixed.\n",
@@ -688,6 +703,13 @@ void kobject_put(struct kobject *kobj)
WARN(1, KERN_WARNING "kobject: '%s' (%p): is not "
"initialized, yet kobject_put() is being "
"called.\n", kobject_name(kobj), kobj);
+ if (kobj->release_debug)
+ printk("XXX kobject_put(%s): ref=%d-- (%pf:%pf:%pf)\n",
+ kobject_name(kobj),
+ atomic_read(&kobj->kref.refcount),
+ __builtin_return_address(1),
+ __builtin_return_address(2),
+ __builtin_return_address(3));
kref_put(&kobj->kref, kobject_release);
}
}
^ permalink raw reply related
* Re: [PATCH] ata: pata_of_platform: using of_property_read_u32() helper
From: Tejun Heo @ 2017-01-24 16:15 UTC (permalink / raw)
To: Kefeng Wang
Cc: linux-kernel, linux-ide, Bartlomiej Zolnierkiewicz, Hans de Goede
In-Reply-To: <1485259709-16487-1-git-send-email-wangkefeng.wang@huawei.com>
On Tue, Jan 24, 2017 at 08:08:29PM +0800, Kefeng Wang wrote:
> Using better of_property_read_u32() than generic of_get_property().
>
> Cc: Bartlomiej Zolnierkiewicz <b.zolnierkie@samsung.com>
> Cc: Hans de Goede <hdegoede@redhat.com>
> Cc: Tejun Heo <tj@kernel.org>
> Signed-off-by: Kefeng Wang <wangkefeng.wang@huawei.com>
Applied to libata/for-4.11.
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-24 15:57 UTC (permalink / raw)
To: Gwendal Grignou
Cc: One Thousand Gnomes, Greg Kroah-Hartman, Sergei Shtylyov,
IDE/ATA development list, whiteheadm
In-Reply-To: <CAMHSBOVS+ehZkHG7nnUS9qQoDXKmsiBUEhBbPb=3qA2-ZaQ2Ow@mail.gmail.com>
Hello,
On Mon, Jan 23, 2017 at 03:36:18PM -0800, Gwendal Grignou wrote:
> > 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?
>
> It was intentional. When I wrote the code, I used other transport as
> inspiration, in particular drivers/scsi/scsi_transport_sas.c.
> For instance, in sas_port_alloc, "port->dev.parent =
> get_device(parent);", undo in sas_port_release().
> From your debug code, in case of ATA at least, it looks unnecessary.
kobjects release their parents on kobject_del() rather than release,
which I don't think is a good design, so if there are code paths which
try to walk up the hierarchy after being deleted but before released,
having the extra ref around release does make sense. I don't know
whether that's the case for the transport code tho.
Anyways, I was curious but this is a bit tangential to the issue.
This code does make the ata device hang around till the transport
kobject is released and the problem is caused by the transport kobject
not being released, so there's linkage there, but we wanna find out
why the transport kobject isn't being released and fix it.
> Moreover, in the error path of ata_XXX_add, a put_device(&link->tdev)
> Also,How ata_tport_add() increase the reference twice before calling
> device_add()?
Dunno, attribute additions?
> > 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.
>
> That's the root cause of pata port not released. Can we run your debug
> code one more time, enabling ap->tdev.kobj.release_debug?
Yeah, I'll update the patch.
Thanks.
--
tejun
^ permalink raw reply
* [PATCH] ata: pata_of_platform: using of_property_read_u32() helper
From: Kefeng Wang @ 2017-01-24 12:08 UTC (permalink / raw)
To: Tejun Heo, linux-kernel
Cc: linux-ide, Kefeng Wang, Bartlomiej Zolnierkiewicz, Hans de Goede
Using better of_property_read_u32() than generic of_get_property().
Cc: Bartlomiej Zolnierkiewicz <b.zolnierkie@samsung.com>
Cc: Hans de Goede <hdegoede@redhat.com>
Cc: Tejun Heo <tj@kernel.org>
Signed-off-by: Kefeng Wang <wangkefeng.wang@huawei.com>
---
drivers/ata/pata_of_platform.c | 9 ++-------
1 file changed, 2 insertions(+), 7 deletions(-)
diff --git a/drivers/ata/pata_of_platform.c b/drivers/ata/pata_of_platform.c
index b6b7af8..201a32d 100644
--- a/drivers/ata/pata_of_platform.c
+++ b/drivers/ata/pata_of_platform.c
@@ -32,7 +32,6 @@ static int pata_of_platform_probe(struct platform_device *ofdev)
unsigned int reg_shift = 0;
int pio_mode = 0;
int pio_mask;
- const u32 *prop;
ret = of_address_to_resource(dn, 0, &io_res);
if (ret) {
@@ -50,13 +49,9 @@ static int pata_of_platform_probe(struct platform_device *ofdev)
irq_res = platform_get_resource(ofdev, IORESOURCE_IRQ, 0);
- prop = of_get_property(dn, "reg-shift", NULL);
- if (prop)
- reg_shift = be32_to_cpup(prop);
+ of_property_read_u32(dn, "reg-shift", ®_shift);
- prop = of_get_property(dn, "pio-mode", NULL);
- if (prop) {
- pio_mode = be32_to_cpup(prop);
+ if (!of_property_read_u32(dn, "pio-mode", &pio_mode)) {
if (pio_mode > 6) {
dev_err(&ofdev->dev, "invalid pio-mode\n");
return -EINVAL;
--
1.7.12.4
^ permalink raw reply related
* Re: [PATCH] pata_legacy: Allow disabling of legacy PATA device probes on non-PCI systems
From: tedheadster @ 2017-01-23 23:57 UTC (permalink / raw)
To: Gwendal Grignou
Cc: Tejun Heo, One Thousand Gnomes, Greg Kroah-Hartman,
Sergei Shtylyov, IDE/ATA development list
In-Reply-To: <CAMHSBOVS+ehZkHG7nnUS9qQoDXKmsiBUEhBbPb=3qA2-ZaQ2Ow@mail.gmail.com>
Gwendal,
>> 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.
> That's the root cause of pata port not released. Can we run your debug
> code one more time, enabling ap->tdev.kobj.release_debug?
>
I'm building the debug kernel because I have the test hardware, but
I'm not quite sure what I need to do to enable
ap->tdev.kobj.release_debug. Tejun's patch enables kobj.release_debug,
but I'm not sure if that is the same thing you want. Please advise.
- Matthew
^ permalink raw reply
* Re: [PATCH] pata_legacy: Allow disabling of legacy PATA device probes on non-PCI systems
From: Gwendal Grignou @ 2017-01-23 23:36 UTC (permalink / raw)
To: Tejun Heo
Cc: One Thousand Gnomes, Greg Kroah-Hartman, Sergei Shtylyov,
IDE/ATA development list, whiteheadm
In-Reply-To: <20170120193828.GB9280@mtj.duckdns.org>
On Fri, Jan 20, 2017 at 11:38 AM, Tejun Heo <tj@kernel.org> wrote:
> 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?
It was intentional. When I wrote the code, I used other transport as
inspiration, in particular drivers/scsi/scsi_transport_sas.c.
For instance, in sas_port_alloc, "port->dev.parent =
get_device(parent);", undo in sas_port_release().
>From your debug code, in case of ATA at least, it looks unnecessary.
Moreover, in the error path of ata_XXX_add, a put_device(&link->tdev)
Also,How ata_tport_add() increase the reference twice before calling
device_add()?
>
> 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.
That's the root cause of pata port not released. Can we run your debug
code one more time, enabling ap->tdev.kobj.release_debug?
Thanks,
Gwendal.
>
> Thanks.
>
> --
> tejun
^ permalink raw reply
* Re: [PATCH v2] libata:pata_atiixp: Don't use unconnected secondary port on SB600/SB700
From: Tejun Heo @ 2017-01-23 19:41 UTC (permalink / raw)
To: Darren Stevens; +Cc: linux-ide
In-Reply-To: <4978cadcbc.52dc8905@auth.smtp.1and1.co.uk>
On Sun, Jan 22, 2017 at 07:37:03PM +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>
Applied to libata/for-4.11.
Thanks.
--
tejun
^ permalink raw reply
* Re: [PATCH v2] libata-sff: Don't scan disabled ports when checking for legacy mode.
From: Tejun Heo @ 2017-01-23 19:41 UTC (permalink / raw)
To: Darren Stevens; +Cc: linux-ide, Sergei Shtylyov
In-Reply-To: <4978ca2860b.2931a5b8@auth.smtp.1and1.co.uk>
Hello,
I did some minor edits and applied the following to libata/for-4.11.
When you're attaching the patch next time, please attach the whole
output of git-format-patch; otherwise, I have to stitch them back
together.
Thanks.
------ 8< ------
>From 589d572671fe7ca342d25cde07a0e310a6912971 Mon Sep 17 00:00:00 2001
From: Darren Stevens <darren@stevens-zone.net>
Date: Mon, 23 Jan 2017 14:33:36 -0500
Subject: [PATCH] libata-sff: Don't scan disabled ports when checking for
legacy mode.
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 south bridge 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'
tj: Addressed Sergei's review points. Other style edits.
Signed-off-by: Darren Stevens <darren@stevens-zone.net>
Signed-off-by: Tejun Heo <tj@kernel.org>
Cc: Sergei Shtylyov <sergei.shtylyov@cogentembedded.com>
---
drivers/ata/libata-sff.c | 16 +++++++++++++---
1 file changed, 13 insertions(+), 3 deletions(-)
diff --git a/drivers/ata/libata-sff.c b/drivers/ata/libata-sff.c
index 4441b5c..2bd92dc 100644
--- a/drivers/ata/libata-sff.c
+++ b/drivers/ata/libata-sff.c
@@ -2428,11 +2428,21 @@ int ata_pci_sff_activate_host(struct ata_host *host,
return rc;
if ((pdev->class >> 8) == PCI_CLASS_STORAGE_IDE) {
- u8 tmp8, mask;
+ u8 tmp8, mask = 0;
- /* 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 are marked as 'dummy' during
+ * this check
+ */
pci_read_config_byte(pdev, PCI_CLASS_PROG, &tmp8);
- mask = (1 << 2) | (1 << 0);
+ if (!ata_port_is_dummy(host->ports[0]))
+ mask |= (1 << 0);
+ if (!ata_port_is_dummy(host->ports[1]))
+ mask |= (1 << 2);
if ((tmp8 & mask) != mask)
legacy_mode = 1;
}
--
2.9.3
^ permalink raw reply related
* [PATCH] pata_octeon_cf: remove unused local variables from octeon_cf_set_piomode()
From: tj @ 2017-01-23 19:31 UTC (permalink / raw)
To: David Binderman
Cc: b.zolnierkie@samsung.com, linux-ide@vger.kernel.org,
linux-kernel@vger.kernel.org
In-Reply-To: <VI1PR08MB1022E4D2E06B02E7B6BFCB5E9C720@VI1PR08MB1022.eurprd08.prod.outlook.com>
>From d786b91f422c6ad4c0d9bb9c1bef2dd5008e3d9d Mon Sep 17 00:00:00 2001
From: Tejun Heo <tj@kernel.org>
Date: Mon, 23 Jan 2017 14:28:51 -0500
@t1 and @t2i are calculated along with @t2 but never used. Drop them.
Signed-off-by: Tejun Heo <tj@kernel.org>
Reported-by: David Binderman <dcb314@hotmail.com>
---
Applied to libata/for-4.11. Thanks.
drivers/ata/pata_octeon_cf.c | 8 --------
1 file changed, 8 deletions(-)
diff --git a/drivers/ata/pata_octeon_cf.c b/drivers/ata/pata_octeon_cf.c
index e94e7ca..f524a90 100644
--- a/drivers/ata/pata_octeon_cf.c
+++ b/drivers/ata/pata_octeon_cf.c
@@ -138,9 +138,7 @@ static void octeon_cf_set_piomode(struct ata_port *ap, struct ata_device *dev)
int trh;
int pause;
/* These names are timing parameters from the ATA spec */
- int t1;
int t2;
- int t2i;
/*
* A divisor value of four will overflow the timing fields at
@@ -154,15 +152,9 @@ static void octeon_cf_set_piomode(struct ata_port *ap, struct ata_device *dev)
BUG_ON(ata_timing_compute(dev, dev->pio_mode, &timing, T, T));
- t1 = timing.setup;
- if (t1)
- t1--;
t2 = timing.active;
if (t2)
t2--;
- t2i = timing.act8b;
- if (t2i)
- t2i--;
trh = ns_to_tim_reg(div, 20);
if (trh)
--
2.9.3
^ permalink raw reply related
* [PATCH v6 14/14] ARM: dts: da850-lcdk: enable the SATA node
From: Bartosz Golaszewski @ 2017-01-23 17:00 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: <1485190856-4711-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 v6 13/14] ARM: dts: da850: add the SATA node
From: Bartosz Golaszewski @ 2017-01-23 17:00 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: <1485190856-4711-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 v6 12/14] ARM: davinci: remove BUG_ON() from da850_register_sata()
From: Bartosz Golaszewski @ 2017-01-23 17:00 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: <1485190856-4711-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 v6 11/14] sata: ahci-da850: un-hardcode the MPY bits
From: Bartosz Golaszewski @ 2017-01-23 17:00 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: <1485190856-4711-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>
Acked-by: Tejun Heo <tj@kernel.org>
---
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 v6 10/14] sata: ahci-da850: add a workaround for controller instability
From: Bartosz Golaszewski @ 2017-01-23 17:00 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: <1485190856-4711-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>
Acked-by: Tejun Heo <tj@kernel.org>
---
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 v6 09/14] sata: ahci: export ahci_do_hardreset() locally
From: Bartosz Golaszewski @ 2017-01-23 17:00 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: <1485190856-4711-1-git-send-email-bgolaszewski@baylibre.com>
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@baylibre.com>
Acked-by: Tejun Heo <tj@kernel.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
^ permalink raw reply related
* [PATCH v6 08/14] sata: ahci-da850: implement a workaround for the softreset quirk
From: Bartosz Golaszewski @ 2017-01-23 17:00 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: <1485190856-4711-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>
Acked-by: Tejun Heo <tj@kernel.org>
---
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 v6 07/14] sata: ahci-da850: add device tree match table
From: Bartosz Golaszewski @ 2017-01-23 17:00 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: <1485190856-4711-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>
Acked-by: Tejun Heo <tj@kernel.org>
---
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 v6 06/14] ARM: davinci: da850: model the SATA refclk
From: Bartosz Golaszewski @ 2017-01-23 17:00 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: <1485190856-4711-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 v6 05/14] ARM: davinci: da850: add con_id for the SATA clock
From: Bartosz Golaszewski @ 2017-01-23 17:00 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: <1485190856-4711-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
* [PATCH v6 04/14] sata: ahci-da850: get the sata clock using a connection id
From: Bartosz Golaszewski @ 2017-01-23 17:00 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: <1485190856-4711-1-git-send-email-bgolaszewski@baylibre.com>
In preparation for using two clocks in the driver (the sysclk2-based
clock and the external REFCLK), check if we got a functional clock
after calling ahci_platform_get_resources(). If not, retry calling
clk_get() with con_id specified.
Signed-off-by: Bartosz Golaszewski <bgolaszewski@baylibre.com>
Acked-by: Tejun Heo <tj@kernel.org>
---
drivers/ata/ahci_da850.c | 16 ++++++++++++++++
1 file changed, 16 insertions(+)
diff --git a/drivers/ata/ahci_da850.c b/drivers/ata/ahci_da850.c
index 267a3d3..8cfdc86 100644
--- a/drivers/ata/ahci_da850.c
+++ b/drivers/ata/ahci_da850.c
@@ -71,12 +71,28 @@ static int ahci_da850_probe(struct platform_device *pdev)
struct ahci_host_priv *hpriv;
struct resource *res;
void __iomem *pwrdn_reg;
+ struct clk *clk;
int rc;
hpriv = ahci_platform_get_resources(pdev);
if (IS_ERR(hpriv))
return PTR_ERR(hpriv);
+ /*
+ * Internally ahci_platform_get_resources() calls clk_get(dev, NULL)
+ * when trying to obtain the first clock. This SATA controller uses
+ * two clocks for which we specify two connection ids. If we don't
+ * have a clock at this point - call clk_get() again with
+ * con_id = "sata".
+ */
+ if (!hpriv->clks[0]) {
+ clk = clk_get(dev, "sata");
+ if (IS_ERR(clk))
+ return PTR_ERR(clk);
+
+ hpriv->clks[0] = clk;
+ }
+
rc = ahci_platform_enable_resources(hpriv);
if (rc)
return rc;
--
2.9.3
^ permalink raw reply related
* [PATCH v6 03/14] ARM: davinci: add a clock lookup entry for the SATA clock
From: Bartosz Golaszewski @ 2017-01-23 17:00 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: <1485190856-4711-1-git-send-email-bgolaszewski@baylibre.com>
This entry is needed for the ahci driver to get a functional clock.
Signed-off-by: Bartosz Golaszewski <bgolaszewski@baylibre.com>
---
arch/arm/mach-davinci/da8xx-dt.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/arch/arm/mach-davinci/da8xx-dt.c b/arch/arm/mach-davinci/da8xx-dt.c
index 9ee44da..b83e5d1 100644
--- a/arch/arm/mach-davinci/da8xx-dt.c
+++ b/arch/arm/mach-davinci/da8xx-dt.c
@@ -42,6 +42,7 @@ static struct of_dev_auxdata da850_auxdata_lookup[] __initdata = {
OF_DEV_AUXDATA("ti,da830-ohci", 0x01e25000, "ohci-da8xx", NULL),
OF_DEV_AUXDATA("ti,da830-musb", 0x01e00000, "musb-da8xx", NULL),
OF_DEV_AUXDATA("ti,da830-usb-phy", 0x01c1417c, "da8xx-usb-phy", NULL),
+ OF_DEV_AUXDATA("ti,da850-ahci", 0x01e18000, "ahci_da850", NULL),
{}
};
--
2.9.3
^ permalink raw reply related
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox