* Re: [PATCH v3 0/5] ATA/ARM: convert ARM/DaVinci to use libata PATA drivers
From: Bartlomiej Zolnierkiewicz @ 2017-03-24 15:38 UTC (permalink / raw)
To: Sekhar Nori
Cc: Tejun Heo, Sergei Shtylyov, Kevin Hilman, Arnd Bergmann,
Russell King, Dmitry Eremin-Solenikov, linux-ide,
linux-arm-kernel, linux-kernel
In-Reply-To: <aaf669ba-4439-9af7-7943-c9163bd99020@ti.com>
On Thursday, March 23, 2017 07:27:05 PM Sekhar Nori wrote:
> On Wednesday 22 March 2017 11:50 PM, Bartlomiej Zolnierkiewicz wrote:
> > Hi,
> >
> > This patchset adds Palmchip BK3710 IDE controller driver to
> > libata and switches ARM/DaVinci to use it (instead of the old
> > IDE driver).
> >
> > Sekhar, please check that it still works after changes, thanks.
>
> Did some basic mount/read/write tests, it passed.
Thanks for checking.
I guess that you can merge the patchset now to your tree
(with Tejun's ACK for the whole patches and Sergei's ACK
for ATA changes).
Best regards,
--
Bartlomiej Zolnierkiewicz
Samsung R&D Institute Poland
Samsung Electronics
^ permalink raw reply
* Re: [PATCH v2 2/2] ata: ahci: add support for DaVinci DM816 SATA controller
From: Bartosz Golaszewski @ 2017-03-24 15:22 UTC (permalink / raw)
To: Tejun Heo
Cc: linux-ide, Michael Turquette, Sergei Shtylyov, Kevin Hilman,
Neil Armstrong, Tony Lindgren, Rob Herring, Mark Rutland,
Patrick Titiano, linux-devicetree, LKML, Bartosz Golaszewski
In-Reply-To: <1489489491-14195-3-git-send-email-bgolaszewski@baylibre.com>
2017-03-14 12:04 GMT+01:00 Bartosz Golaszewski <bgolaszewski@baylibre.com>:
> This SATA controller is quite similar to the one present on the DA850
> SoC, but the PHY configuration is different and it supports two HBA
> ports.
>
> The IP suffers from the same PMP issue the DA850 does - if we enable
> PMP but don't use it - softreset fails. Appropriate workaround was
> implemented in this driver as well.
>
> Signed-off-by: Bartosz Golaszewski <bgolaszewski@baylibre.com>
> ---
> drivers/ata/Kconfig | 9 +++
> drivers/ata/Makefile | 1 +
> drivers/ata/ahci_dm816.c | 200 +++++++++++++++++++++++++++++++++++++++++++++++
> 3 files changed, 210 insertions(+)
> create mode 100644 drivers/ata/ahci_dm816.c
>
Hi Tejun,
the DT bindings have been acked by Rob Herring and Tony Lindgren has
picked up the arch/arm patches. If there are no objections, could you
merge this driver and DT bindings for 4.12?
Thanks,
Bartosz
^ permalink raw reply
* ata: WARNING in ata_bmdma_qc_issue
From: Dmitry Vyukov @ 2017-03-24 14:14 UTC (permalink / raw)
To: Tejun Heo, linux-ide, LKML; +Cc: syzkaller
Hello,
The following program triggers WARNING in ata_bmdma_qc_issue:
https://gist.githubusercontent.com/dvyukov/7497a05853e58598b74a2c48030c41f4/raw/38885f75eb05f72bb47fff9c9de657728f529927/gistfile1.txt
------------[ cut here ]------------
WARNING: CPU: 3 PID: 2963 at drivers/ata/libata-sff.c:2827
ata_bmdma_qc_issue+0x2bb/0x5b0 drivers/ata/libata-sff.c:2827
CPU: 3 PID: 2963 Comm: a.out Not tainted 4.11.0-rc3+ #367
Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS Bochs 01/01/2011
Call Trace:
__dump_stack lib/dump_stack.c:16 [inline]
dump_stack+0x1b8/0x28d lib/dump_stack.c:52
panic+0x20c/0x423 kernel/panic.c:180
__warn+0x1c4/0x1e0 kernel/panic.c:541
warn_slowpath_null+0x2c/0x40 kernel/panic.c:584
ata_bmdma_qc_issue+0x2bb/0x5b0 drivers/ata/libata-sff.c:2827
ata_qc_issue+0x6f2/0x1040 drivers/ata/libata-core.c:5335
ata_scsi_translate+0x34a/0x5e0 drivers/ata/libata-scsi.c:2021
__ata_scsi_queuecmd drivers/ata/libata-scsi.c:4249 [inline]
ata_scsi_queuecmd+0x2ae/0x660 drivers/ata/libata-scsi.c:4298
scsi_dispatch_cmd+0x43e/0xb70 drivers/scsi/scsi_lib.c:1665
scsi_request_fn+0x13dc/0x1ec0 drivers/scsi/scsi_lib.c:1800
__blk_run_queue_uncond block/blk-core.c:305 [inline]
__blk_run_queue+0xe3/0x150 block/blk-core.c:323
blk_execute_rq_nowait+0x21c/0x370 block/blk-exec.c:79
sg_common_write.isra.19+0x1057/0x1ac0 drivers/scsi/sg.c:803
sg_write+0x78a/0xd40 drivers/scsi/sg.c:679
__vfs_write+0xfb/0x890 fs/read_write.c:508
vfs_write+0x187/0x530 fs/read_write.c:558
SYSC_write fs/read_write.c:605 [inline]
SyS_write+0xef/0x220 fs/read_write.c:597
entry_SYSCALL_64_fastpath+0x1f/0xc2
RIP: 0033:0x4469d9
RSP: 002b:00007fff3adf5a58 EFLAGS: 00000293 ORIG_RAX: 0000000000000001
RAX: ffffffffffffffda RBX: 00000000004002c8 RCX: 00000000004469d9
RDX: 0000000000000090 RSI: 0000000020012000 RDI: 0000000000000003
RBP: 0000000000000086 R08: 0000000000000000 R09: 0000000000000000
R10: 0000000000000000 R11: 0000000000000293 R12: 0000000000000000
R13: 0000000000402c00 R14: 0000000000402c90 R15: 0000000000000000
On commit ebe64824e9de4b3ab3bd3928312b4b2bc57b4b7e
Looks similar to the WARNING in ata_sff_qc_issue:
https://groups.google.com/forum/#!topic/syzkaller/0v1qHkmM-VU
Should we also remove the WARNING in ata_bmdma_qc_issue?
^ permalink raw reply
* Re: [PATCH v2 0/4] ARM: dm8168-evm: add SATA support
From: Tony Lindgren @ 2017-03-23 21:00 UTC (permalink / raw)
To: Bartosz Golaszewski
Cc: Rob Herring, Mark Rutland, Neil Armstrong, Michael Turquette,
Kevin Hilman, Patrick Titiano, Paul Walmsley, Sergei Shtylyov,
linux-ide-u79uwXL29TY76Z2rM5mHXA,
devicetree-u79uwXL29TY76Z2rM5mHXA,
linux-kernel-u79uwXL29TY76Z2rM5mHXA,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
linux-omap-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <1489490055-2318-1-git-send-email-bgolaszewski-rdvid1DuHRBWk0Htik3J/w@public.gmane.org>
* Bartosz Golaszewski <bgolaszewski-rdvid1DuHRBWk0Htik3J/w@public.gmane.org> [170314 04:16]:
> This series implements support for the on-board SATA controller
> on the dm8168-evm board.
>
> The first patch adds the clock domain and hwmod entries for the
> SATA module.
>
> The second patch models the external reference clock used by SATA.
>
> The third adds the SATA node to the SoC's device tree.
>
> The fourth enables the SATA driver by default in omap2plus_defconfig.
>
> Tested together with the series implementing the driver for
> ahci-dm816 on a DM8168-EVM board.
>
> v1 -> v2:
> PATCH 1/4:
> - removed the unused define from cm81xx.h
> PATCH 3/4:
> - fixed the size of the mapped register region
>
> Bartosz Golaszewski (3):
> ARM: dts: dm8168-evm: add the external reference clock for SATA
> ARM: dts: dm8168-evm: add SATA node
> ARM: omap2plus_defconfig: enable ahci-dm816 module
>
> Kevin Hilman (1):
> ARM: OMAP2+: dm81xx: Add clkdm and hwmod for SATA
Applying all for for v4.12 into soc, dts-v2 and defconfig
branches.
Thanks,
Tony
--
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: support ranges TRIM for libata
From: Martin K. Petersen @ 2017-03-23 15:39 UTC (permalink / raw)
To: Christoph Hellwig
Cc: James Bottomley, Tejun Heo, martin.petersen, axboe, linux-ide,
linux-scsi, linux-block
In-Reply-To: <20170323153001.GA32484@lst.de>
Christoph Hellwig <hch@lst.de> writes:
> Meh, I remember why I gave up on it - to support queued trim passthrough
> we'd need to implement ATA 32 for the auxiliary fields and thus support
> 32-bit CDBs. I don't really want to go there..
I wish we could stick with SATL. However, I have attempted this a few
times over the years and I am with Christoph here. Due to the
discrepancies between ACS and SBC it's a twisted mess. And the iterative
approach has not worked well for HBA SATLs either. Quite the contrary.
So I think sticking the DSM TRIM payload onto a SCSI command is the path
of least resistance. And then NAK'ing attempts to issue these commands
through sg.
I'll take closer look at the entire series tomorrow or Monday. I want to
test multiple ranges on my "Little Shop of SSD Horrors" when I get back
home from LSF/MM.
--
Martin K. Petersen Oracle Linux Engineering
^ permalink raw reply
* Re: support ranges TRIM for libata
From: Christoph Hellwig @ 2017-03-23 15:30 UTC (permalink / raw)
To: James Bottomley
Cc: Christoph Hellwig, Tejun Heo, martin.petersen, axboe, linux-ide,
linux-scsi, linux-block
In-Reply-To: <20170323144330.GA31447@lst.de>
On Thu, Mar 23, 2017 at 03:43:30PM +0100, Christoph Hellwig wrote:
> I tried this earlier before giving up on it because it looked to ugly.
> But I can complete that version of it and post it for people to compare.
Meh, I remember why I gave up on it - to support queued trim passthrough
we'd need to implement ATA 32 for the auxiliary fields and thus support
32-bit CDBs. I don't really want to go there..
^ permalink raw reply
* Re: support ranges TRIM for libata
From: Christoph Hellwig @ 2017-03-23 15:27 UTC (permalink / raw)
To: Tejun Heo
Cc: Christoph Hellwig, James Bottomley, martin.petersen, axboe,
linux-ide, linux-scsi, linux-block
In-Reply-To: <20170323150458.GA3241@htj.duckdns.org>
On Thu, Mar 23, 2017 at 11:04:58AM -0400, Tejun Heo wrote:
> I kinda like the idea of sticking with satl as that's how libata has
> been doing most things even if the implementation is uglier. It'd be
> great to find out whether the ugliness would be acceptable or too
> much.
The SATL way is simply broken. I'll just leave the code corrupting
user data and 5 times slower than it could be then.
^ permalink raw reply
* Re: support ranges TRIM for libata
From: Tejun Heo @ 2017-03-23 15:04 UTC (permalink / raw)
To: Christoph Hellwig
Cc: James Bottomley, martin.petersen, axboe, linux-ide, linux-scsi,
linux-block
In-Reply-To: <20170323144330.GA31447@lst.de>
Hello, Christoph.
On Thu, Mar 23, 2017 at 03:43:30PM +0100, Christoph Hellwig wrote:
> > That's up to you ... from the point of view of code documenting itself,
> > forming the ATA_16 TRIM in sd and not doing any satl transformation is
> > easier for others to follow, but if it's going to cause more code, I'm
> > only marginal on the advantages of easier to follow code.
>
> I tried this earlier before giving up on it because it looked to ugly.
> But I can complete that version of it and post it for people to compare.
I kinda like the idea of sticking with satl as that's how libata has
been doing most things even if the implementation is uglier. It'd be
great to find out whether the ugliness would be acceptable or too
much.
Thanks.
--
tejun
^ permalink raw reply
* Re: support ranges TRIM for libata
From: Christoph Hellwig @ 2017-03-23 14:43 UTC (permalink / raw)
To: James Bottomley
Cc: Christoph Hellwig, Tejun Heo, martin.petersen, axboe, linux-ide,
linux-scsi, linux-block
In-Reply-To: <1490279706.2202.17.camel@HansenPartnership.com>
On Thu, Mar 23, 2017 at 10:35:06AM -0400, James Bottomley wrote:
> I'm certainly not saying we blindly follow t10, but I believe their
> intent is to issue the next command from the completion of the first
> (we can do this using qc->complete_fn, like atapi_request_sense). That
> way we don't get any tag problems because there's only one command
> outstanding at once; reusing the qc means no allocation issues either.
>
> The t10 approach does mean the SG_IO problem is actually fixable rather
> than simply erroring out.
It would be sort of fixable, but with a lot of hackery.
> That's up to you ... from the point of view of code documenting itself,
> forming the ATA_16 TRIM in sd and not doing any satl transformation is
> easier for others to follow, but if it's going to cause more code, I'm
> only marginal on the advantages of easier to follow code.
I tried this earlier before giving up on it because it looked to ugly.
But I can complete that version of it and post it for people to compare.
^ permalink raw reply
* Re: support ranges TRIM for libata
From: James Bottomley @ 2017-03-23 14:35 UTC (permalink / raw)
To: Christoph Hellwig
Cc: Tejun Heo, martin.petersen, axboe, linux-ide, linux-scsi,
linux-block
In-Reply-To: <20170323135521.GA30361@lst.de>
On Thu, 2017-03-23 at 14:55 +0100, Christoph Hellwig wrote:
> On Thu, Mar 23, 2017 at 09:47:41AM -0400, James Bottomley wrote:
> > > The current implementation already has the issue of that it does
> > > corrupt user data reliably if the using SG_IO for WRITE SAME
> > > commands.
> >
> > That does need fixing.
>
> I don't think it's fixable as long as we translate the data payload.
>
> > Why can't we do what the t10 sat document recommends: if the ATA
> > device doesn't support the XL version (32 bit ranges) then
> > translate unmap to multiple non-XL commands?
>
> Because libata sits underneath the tag allocator we'd getting into
> a giant set of problems. Where do you expect the new commands to
> magically come from? And adhere to the queueing limits and not
> actually deadlock in one way or another?
I'm certainly not saying we blindly follow t10, but I believe their
intent is to issue the next command from the completion of the first
(we can do this using qc->complete_fn, like atapi_request_sense). That
way we don't get any tag problems because there's only one command
outstanding at once; reusing the qc means no allocation issues either.
The t10 approach does mean the SG_IO problem is actually fixable rather
than simply erroring out.
> > I don't necessarily object to the vendor specific 1<->1 approach,
> > it's just it won't fix the problem you cited above (SG_IO WRITE
> > SAME), its just that now we error the command, which may cause some
> > surprise.
>
> We now error WRITE SAME for passthrough consistently. Before we only
> accepted it only with the unmap bit set, and even did so incorrectly
> (not checking that the payload was all zeros).
If it never worked for anyone, I'm OK with changing it to error out.
> > I also wonder if we couldn't simply do an ATA_16 TRIM if we're
> > already going to all the trouble of recognising ATA devices in the
> > sd discard path?
>
> We probably could do that as well. But that would drag a lot more
> ATA-specific code into sd.c than just formatting the ranges.
That's up to you ... from the point of view of code documenting itself,
forming the ATA_16 TRIM in sd and not doing any satl transformation is
easier for others to follow, but if it's going to cause more code, I'm
only marginal on the advantages of easier to follow code.
James
^ permalink raw reply
* Re: [PATCH v3 0/5] ATA/ARM: convert ARM/DaVinci to use libata PATA drivers
From: Sekhar Nori @ 2017-03-23 13:57 UTC (permalink / raw)
To: Bartlomiej Zolnierkiewicz, Tejun Heo
Cc: Russell King, Sergei Shtylyov, Arnd Bergmann,
Dmitry Eremin-Solenikov, Kevin Hilman, linux-kernel, linux-ide,
linux-arm-kernel
In-Reply-To: <1490206862-26065-1-git-send-email-b.zolnierkie@samsung.com>
On Wednesday 22 March 2017 11:50 PM, Bartlomiej Zolnierkiewicz wrote:
> Hi,
>
> This patchset adds Palmchip BK3710 IDE controller driver to
> libata and switches ARM/DaVinci to use it (instead of the old
> IDE driver).
>
> Sekhar, please check that it still works after changes, thanks.
Did some basic mount/read/write tests, it passed.
Thanks,
Sekhar
^ permalink raw reply
* Re: support ranges TRIM for libata
From: Christoph Hellwig @ 2017-03-23 13:55 UTC (permalink / raw)
To: James Bottomley
Cc: Christoph Hellwig, Tejun Heo, martin.petersen, axboe, linux-ide,
linux-scsi, linux-block
In-Reply-To: <1490276861.2202.8.camel@HansenPartnership.com>
On Thu, Mar 23, 2017 at 09:47:41AM -0400, James Bottomley wrote:
> > The current implementation already has the issue of that it does
> > corrupt user data reliably if the using SG_IO for WRITE SAME
> > commands.
>
> That does need fixing.
I don't think it's fixable as long as we translate the data payload.
> Why can't we do what the t10 sat document recommends: if the ATA device
> doesn't support the XL version (32 bit ranges) then translate unmap to
> multiple non-XL commands?
Because libata sits underneath the tag allocator we'd getting into
a giant set of problems. Where do you expect the new commands to
magically come from? And adhere to the queueing limits and not actually
deadlock in one way or another?
> I don't necessarily object to the vendor specific 1<->1 approach, it's
> just it won't fix the problem you cited above (SG_IO WRITE SAME), its
> just that now we error the command, which may cause some surprise.
We now error WRITE SAME for passthrough consistently. Before we only
accepted it only with the unmap bit set, and even did so incorrectly
(not checking that the payload was all zeros).
> I
> also wonder if we couldn't simply do an ATA_16 TRIM if we're already
> going to all the trouble of recognising ATA devices in the sd discard
> path?
We probably could do that as well. But that would drag a lot more
ATA-specific code into sd.c than just formatting the ranges.
^ permalink raw reply
* Re: support ranges TRIM for libata
From: James Bottomley @ 2017-03-23 13:47 UTC (permalink / raw)
To: Christoph Hellwig, Tejun Heo
Cc: martin.petersen, axboe, linux-ide, linux-scsi, linux-block
In-Reply-To: <20170322181947.GA4733@lst.de>
On Wed, 2017-03-22 at 19:19 +0100, Christoph Hellwig wrote:
> On Tue, Mar 21, 2017 at 02:59:01PM -0400, Tejun Heo wrote:
> > I do like the fact that this is a lot simpler than the previous
> > implementation but am not quite sure we want to deviate
> > significantly from what we do for other commands (command
> > translation). Is it because fixing the existing implementation
> > would involve invaisve changes including memory allocations?
>
> The current implementation already has the issue of that it does
> corrupt user data reliably if the using SG_IO for WRITE SAME
> commands.
That does need fixing.
> Doing ranges using translation would turn into a nightmare because
> ATA TRIM ranges are 16 bits long while SCSI UNAMP ranges are 32-bit,
> so we effectively can't translated them without introducing a
> non-standard hook between libata and scsi to communicate that
> limit.
Why can't we do what the t10 sat document recommends: if the ATA device
doesn't support the XL version (32 bit ranges) then translate unmap to
multiple non-XL commands?
I don't necessarily object to the vendor specific 1<->1 approach, it's
just it won't fix the problem you cited above (SG_IO WRITE SAME), its
just that now we error the command, which may cause some surprise. I
also wonder if we couldn't simply do an ATA_16 TRIM if we're already
going to all the trouble of recognising ATA devices in the sd discard
path?
James
> And once we're down that path we might as well just do the
> right thing directly.
^ permalink raw reply
* ata: WARNING in ata_qc_issue
From: Dmitry Vyukov @ 2017-03-23 13:25 UTC (permalink / raw)
To: Tejun Heo, linux-ide, LKML; +Cc: syzkaller
Hello,
The following program triggers WARNING in ata_qc_issue:
https://gist.githubusercontent.com/dvyukov/3503afce181b7d48dabb421e10e70b00/raw/d049bd2128a8b1089497beb6104ba48c5550b4a8/gistfile1.txt
------------[ cut here ]------------
WARNING: CPU: 3 PID: 2956 at drivers/ata/libata-core.c:5317
ata_qc_issue+0xd14/0x1040 drivers/ata/libata-core.c:5316
CPU: 3 PID: 2956 Comm: a.out Not tainted 4.11.0-rc3+ #365
Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS Bochs 01/01/2011
Call Trace:
__dump_stack lib/dump_stack.c:16 [inline]
dump_stack+0x1b8/0x28d lib/dump_stack.c:52
panic+0x20c/0x423 kernel/panic.c:180
__warn+0x1c4/0x1e0 kernel/panic.c:541
warn_slowpath_null+0x2c/0x40 kernel/panic.c:584
ata_qc_issue+0xd14/0x1040 drivers/ata/libata-core.c:5316
ata_scsi_translate+0x34a/0x5e0 drivers/ata/libata-scsi.c:2021
__ata_scsi_queuecmd drivers/ata/libata-scsi.c:4249 [inline]
ata_scsi_queuecmd+0x2ae/0x660 drivers/ata/libata-scsi.c:4298
scsi_dispatch_cmd+0x43e/0xb70 drivers/scsi/scsi_lib.c:1665
scsi_request_fn+0x13dc/0x1ec0 drivers/scsi/scsi_lib.c:1800
__blk_run_queue_uncond block/blk-core.c:305 [inline]
__blk_run_queue+0xe3/0x150 block/blk-core.c:323
__elv_add_request+0x494/0xce0 block/elevator.c:677
blk_execute_rq_nowait+0x214/0x370 block/blk-exec.c:78
blk_execute_rq+0x1ca/0x280 block/blk-exec.c:103
sg_scsi_ioctl+0x3d0/0x7f0 block/scsi_ioctl.c:510
sg_ioctl+0x1fdd/0x2f00 drivers/scsi/sg.c:1075
vfs_ioctl fs/ioctl.c:45 [inline]
do_vfs_ioctl+0x1af/0x16d0 fs/ioctl.c:685
SYSC_ioctl fs/ioctl.c:700 [inline]
SyS_ioctl+0x8f/0xc0 fs/ioctl.c:691
entry_SYSCALL_64_fastpath+0x1f/0xc2
RIP: 0033:0x434cd9
RSP: 002b:00007ffc5cd86308 EFLAGS: 00000286 ORIG_RAX: 0000000000000010
RAX: ffffffffffffffda RBX: 00000000004002b0 RCX: 0000000000434cd9
RDX: 0000000020001000 RSI: 0000000000000001 RDI: 0000000000000003
RBP: 0000000000000086 R08: 0000000000000000 R09: 0000000000000000
R10: 0000000000000000 R11: 0000000000000286 R12: 0000000000000000
R13: 0000000000401a30 R14: 0000000000401ac0 R15: 0000000000000000
On commit 093b995e3b55a0ae0670226ddfcb05bfbf0099ae
^ permalink raw reply
* Re: [PATCH v3 3/5] pata_bk3710: clear status bits of BMISP on chipset initialization
From: Sergei Shtylyov @ 2017-03-23 10:39 UTC (permalink / raw)
To: Bartlomiej Zolnierkiewicz, Tejun Heo
Cc: Sekhar Nori, Kevin Hilman, Arnd Bergmann, Russell King,
Dmitry Eremin-Solenikov, linux-ide, linux-arm-kernel,
linux-kernel
In-Reply-To: <1490206862-26065-4-git-send-email-b.zolnierkie@samsung.com>
On 3/22/2017 9:21 PM, Bartlomiej Zolnierkiewicz wrote:
> Clear IORDYINT, INTRSTAT and DMAERROR bits of BMISP register
> (value '1' needs to be written to the bit to clear it).
>
> Suggested-by: Sergei Shtylyov <sergei.shtylyov@cogentembedded.com>
> Signed-off-by: Bartlomiej Zolnierkiewicz <b.zolnierkie@samsung.com>
[...]
Acked-by: Sergei Shtylyov <sergei.shtylyov@cogentembedded.com>
MBR, Sergei
^ permalink raw reply
* Re: [PATCH v3 2/5] pata_bk3710: disable IORDY Timer on chipset initialization
From: Sergei Shtylyov @ 2017-03-23 10:38 UTC (permalink / raw)
To: Bartlomiej Zolnierkiewicz, Tejun Heo
Cc: Russell King, Arnd Bergmann, Dmitry Eremin-Solenikov,
Kevin Hilman, Sekhar Nori, linux-kernel, linux-ide,
linux-arm-kernel
In-Reply-To: <1490206862-26065-3-git-send-email-b.zolnierkie@samsung.com>
On 3/22/2017 9:20 PM, Bartlomiej Zolnierkiewicz wrote:
> Disable IORDY Timer as the driver doesn't handle IORDY Timer
> interrupt anyway.
>
> Suggested-by: Sergei Shtylyov <sergei.shtylyov@cogentembedded.com>
> Signed-off-by: Bartlomiej Zolnierkiewicz <b.zolnierkie@samsung.com>
Acked-by: Sergei Shtylyov <sergei.shtylyov@cogentembedded.com>
MBR, Sergei
^ permalink raw reply
* Re: [PATCH v3 1/5] ata: add Palmchip BK3710 PATA controller driver
From: Sergei Shtylyov @ 2017-03-23 10:37 UTC (permalink / raw)
To: Bartlomiej Zolnierkiewicz, Tejun Heo
Cc: Sekhar Nori, Kevin Hilman, Arnd Bergmann, Russell King,
Dmitry Eremin-Solenikov, linux-ide, linux-arm-kernel,
linux-kernel
In-Reply-To: <1490206862-26065-2-git-send-email-b.zolnierkie@samsung.com>
Hello!
On 3/22/2017 9:20 PM, Bartlomiej Zolnierkiewicz wrote:
> Add Palmchip BK3710 PATA controller driver.
>
> Signed-off-by: Bartlomiej Zolnierkiewicz <b.zolnierkie@samsung.com>
[...]
> diff --git a/drivers/ata/pata_bk3710.c b/drivers/ata/pata_bk3710.c
> new file mode 100644
> index 0000000..1a1223f
> --- /dev/null
> +++ b/drivers/ata/pata_bk3710.c
> @@ -0,0 +1,382 @@
[...]
> +static int __init pata_bk3710_probe(struct platform_device *pdev)
> +{
> + struct clk *clk;
> + struct resource *mem;
> + struct ata_host *host;
> + struct ata_port *ap;
> + void __iomem *base;
> + unsigned long rate;
> + int irq;
> +
> + clk = devm_clk_get(&pdev->dev, NULL);
> + if (IS_ERR(clk))
> + return -ENODEV;
> +
> + clk_enable(clk);
> + rate = clk_get_rate(clk);
> + if (!rate)
> + return -EINVAL;
> +
> + /* NOTE: round *down* to meet minimum timings; we count in clocks */
> + ideclk_period = 1000000000UL / rate;
> +
> + mem = platform_get_resource(pdev, IORESOURCE_MEM, 0);
I'd put this close to devm_iorempe_resource() and would probably even get
rid of 'mem' altogether...
[...]
Acked-by: Sergei Shtylyov <sergei.shtylyov@cogentembedded.com>
MBR, Sergei
^ permalink raw reply
* [PATCH v3 4/5] ARM: davinci: add pata_bk3710 libata driver support
From: Bartlomiej Zolnierkiewicz @ 2017-03-22 18:21 UTC (permalink / raw)
To: Tejun Heo
Cc: Sekhar Nori, Sergei Shtylyov, Kevin Hilman, Arnd Bergmann,
Russell King, Dmitry Eremin-Solenikov, linux-ide,
linux-arm-kernel, linux-kernel, b.zolnierkie
In-Reply-To: <1490206862-26065-1-git-send-email-b.zolnierkie@samsung.com>
From: Sekhar Nori <nsekhar@ti.com>
Signed-off-by: Sekhar Nori <nsekhar@ti.com>
[b.zolnierkie: split from bigger patch + preserved old driver support]
Signed-off-by: Bartlomiej Zolnierkiewicz <b.zolnierkie@samsung.com>
---
arch/arm/mach-davinci/board-dm644x-evm.c | 3 ++-
arch/arm/mach-davinci/board-dm646x-evm.c | 3 ++-
arch/arm/mach-davinci/board-neuros-osd2.c | 3 ++-
3 files changed, 6 insertions(+), 3 deletions(-)
diff --git a/arch/arm/mach-davinci/board-dm644x-evm.c b/arch/arm/mach-davinci/board-dm644x-evm.c
index 023480b..20f1874 100644
--- a/arch/arm/mach-davinci/board-dm644x-evm.c
+++ b/arch/arm/mach-davinci/board-dm644x-evm.c
@@ -744,7 +744,8 @@ static int davinci_phy_fixup(struct phy_device *phydev)
return 0;
}
-#define HAS_ATA IS_ENABLED(CONFIG_BLK_DEV_PALMCHIP_BK3710)
+#define HAS_ATA (IS_ENABLED(CONFIG_BLK_DEV_PALMCHIP_BK3710) || \
+ IS_ENABLED(CONFIG_PATA_BK3710))
#define HAS_NOR IS_ENABLED(CONFIG_MTD_PHYSMAP)
diff --git a/arch/arm/mach-davinci/board-dm646x-evm.c b/arch/arm/mach-davinci/board-dm646x-evm.c
index f702d4f..cb17682 100644
--- a/arch/arm/mach-davinci/board-dm646x-evm.c
+++ b/arch/arm/mach-davinci/board-dm646x-evm.c
@@ -119,7 +119,8 @@
},
};
-#define HAS_ATA IS_ENABLED(CONFIG_BLK_DEV_PALMCHIP_BK3710)
+#define HAS_ATA (IS_ENABLED(CONFIG_BLK_DEV_PALMCHIP_BK3710) || \
+ IS_ENABLED(CONFIG_PATA_BK3710))
#ifdef CONFIG_I2C
/* CPLD Register 0 bits to control ATA */
diff --git a/arch/arm/mach-davinci/board-neuros-osd2.c b/arch/arm/mach-davinci/board-neuros-osd2.c
index 0a78388..0c02aaa 100644
--- a/arch/arm/mach-davinci/board-neuros-osd2.c
+++ b/arch/arm/mach-davinci/board-neuros-osd2.c
@@ -163,7 +163,8 @@ static void __init davinci_ntosd2_map_io(void)
.wires = 4,
};
-#define HAS_ATA IS_ENABLED(CONFIG_BLK_DEV_PALMCHIP_BK3710)
+#define HAS_ATA (IS_ENABLED(CONFIG_BLK_DEV_PALMCHIP_BK3710) || \
+ IS_ENABLED(CONFIG_PATA_BK3710))
#define HAS_NAND IS_ENABLED(CONFIG_MTD_NAND_DAVINCI)
--
1.9.1
^ permalink raw reply related
* [PATCH v3 3/5] pata_bk3710: clear status bits of BMISP on chipset initialization
From: Bartlomiej Zolnierkiewicz @ 2017-03-22 18:21 UTC (permalink / raw)
To: Tejun Heo
Cc: Sekhar Nori, Sergei Shtylyov, Kevin Hilman, Arnd Bergmann,
Russell King, Dmitry Eremin-Solenikov, linux-ide,
linux-arm-kernel, linux-kernel, b.zolnierkie
In-Reply-To: <1490206862-26065-1-git-send-email-b.zolnierkie@samsung.com>
Clear IORDYINT, INTRSTAT and DMAERROR bits of BMISP register
(value '1' needs to be written to the bit to clear it).
Suggested-by: Sergei Shtylyov <sergei.shtylyov@cogentembedded.com>
Signed-off-by: Bartlomiej Zolnierkiewicz <b.zolnierkie@samsung.com>
---
drivers/ata/pata_bk3710.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/ata/pata_bk3710.c b/drivers/ata/pata_bk3710.c
index f22e4bf..6c3bd5f 100644
--- a/drivers/ata/pata_bk3710.c
+++ b/drivers/ata/pata_bk3710.c
@@ -276,7 +276,7 @@ static void pata_bk3710_chipinit(void __iomem *base)
* (ATA_BMISP_INTRSTAT , CLEAR) |
* (ATA_BMISP_DMAERROR , CLEAR)
*/
- iowrite16(0, base + BK3710_BMISP);
+ iowrite16(0xE, base + BK3710_BMISP);
pata_bk3710_setpiomode(base, NULL, 0, 600, 0);
pata_bk3710_setpiomode(base, NULL, 1, 600, 0);
--
1.9.1
^ permalink raw reply related
* [PATCH v3 2/5] pata_bk3710: disable IORDY Timer on chipset initialization
From: Bartlomiej Zolnierkiewicz @ 2017-03-22 18:20 UTC (permalink / raw)
To: Tejun Heo
Cc: Sekhar Nori, Sergei Shtylyov, Kevin Hilman, Arnd Bergmann,
Russell King, Dmitry Eremin-Solenikov, linux-ide,
linux-arm-kernel, linux-kernel, b.zolnierkie
In-Reply-To: <1490206862-26065-1-git-send-email-b.zolnierkie@samsung.com>
Disable IORDY Timer as the driver doesn't handle IORDY Timer
interrupt anyway.
Suggested-by: Sergei Shtylyov <sergei.shtylyov@cogentembedded.com>
Signed-off-by: Bartlomiej Zolnierkiewicz <b.zolnierkie@samsung.com>
---
drivers/ata/pata_bk3710.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/ata/pata_bk3710.c b/drivers/ata/pata_bk3710.c
index 1a1223f..f22e4bf 100644
--- a/drivers/ata/pata_bk3710.c
+++ b/drivers/ata/pata_bk3710.c
@@ -264,9 +264,9 @@ static void pata_bk3710_chipinit(void __iomem *base)
/*
* IORDYTMP IORDY Timer for Primary Register
- * (ATA_IORDYTMP_IORDYTMP , 0xffff )
+ * (ATA_IORDYTMP_IORDYTMP , DISABLE)
*/
- iowrite32(0xFFFF, base + BK3710_IORDYTMP);
+ iowrite32(0, base + BK3710_IORDYTMP);
/*
* Configure BMISP Register
--
1.9.1
^ permalink raw reply related
* [PATCH v3 0/5] ATA/ARM: convert ARM/DaVinci to use libata PATA drivers
From: Bartlomiej Zolnierkiewicz @ 2017-03-22 18:20 UTC (permalink / raw)
To: Tejun Heo
Cc: Sekhar Nori, Sergei Shtylyov, Kevin Hilman, Arnd Bergmann,
Russell King, Dmitry Eremin-Solenikov, linux-ide,
linux-arm-kernel, linux-kernel, b.zolnierkie
In-Reply-To: <CGME20170322182127epcas1p267b13e8c92205223458863783bdea476@epcas1p2.samsung.com>
Hi,
This patchset adds Palmchip BK3710 IDE controller driver to
libata and switches ARM/DaVinci to use it (instead of the old
IDE driver).
Sekhar, please check that it still works after changes, thanks.
Changes since v2
(https://www.spinics.net/lists/arm-kernel/msg568597.html):
- fixed 'checkpatch.pl --strict' issues reported by Sekhar
- added chipset initialization fixups in the new patches #2-3
(based on the review comments from Sergei)
Changes since v1
(https://www.spinics.net/lists/arm-kernel/msg567442.html):
- addressed review comments from Sergei Shtylyov
- fixed cycle_time unitialized variable issue
Changes since v0.1 draft patch version
(https://www.spinics.net/lists/arm-kernel/msg566932.html):
- fixed cycle_time build warning
- added platform support fixes from Sekhar
- added defconfig changes from Sekhar
- preserved platform support for the old IDE driver
- split it on 3 patches
Best regards,
--
Bartlomiej Zolnierkiewicz
Samsung R&D Institute Poland
Samsung Electronics
Bartlomiej Zolnierkiewicz (3):
ata: add Palmchip BK3710 PATA controller driver
pata_bk3710: disable IORDY Timer on chipset initialization
pata_bk3710: clear status bits of BMISP on chipset initialization
Sekhar Nori (2):
ARM: davinci: add pata_bk3710 libata driver support
ARM: davinci_all_defconfig: convert to use libata PATA
arch/arm/configs/davinci_all_defconfig | 4 +-
arch/arm/mach-davinci/board-dm644x-evm.c | 3 +-
arch/arm/mach-davinci/board-dm646x-evm.c | 3 +-
arch/arm/mach-davinci/board-neuros-osd2.c | 3 +-
drivers/ata/Kconfig | 9 +
drivers/ata/Makefile | 1 +
drivers/ata/pata_bk3710.c | 382 ++++++++++++++++++++++++++++++
7 files changed, 399 insertions(+), 6 deletions(-)
create mode 100644 drivers/ata/pata_bk3710.c
--
1.9.1
^ permalink raw reply
* [PATCH v3 5/5] ARM: davinci_all_defconfig: convert to use libata PATA
From: Bartlomiej Zolnierkiewicz @ 2017-03-22 18:21 UTC (permalink / raw)
To: Tejun Heo
Cc: Russell King, Sergei Shtylyov, Arnd Bergmann, b.zolnierkie,
Dmitry Eremin-Solenikov, Kevin Hilman, Sekhar Nori, linux-kernel,
linux-ide, linux-arm-kernel
In-Reply-To: <1490206862-26065-1-git-send-email-b.zolnierkie@samsung.com>
From: Sekhar Nori <nsekhar@ti.com>
IDE subsystem has been deprecated since 2009 and the majority
(if not all) of Linux distributions have switched to use
libata for ATA support exclusively. However there are still
some users (mostly old or/and embedded non-x86 systems) that
have not converted from using IDE subsystem to libata PATA
drivers. This doesn't seem to be good thing in the long-term
for Linux as while there is less and less PATA systems left
in use:
* testing efforts are divided between two subsystems
* having duplicate drivers for same hardware confuses users
This patch converts davinci_all_defconfig to use libata PATA
drivers.
Signed-off-by: Sekhar Nori <nsekhar@ti.com>
[b.zolnierkie: split from bigger patch + added patch description]
Signed-off-by: Bartlomiej Zolnierkiewicz <b.zolnierkie@samsung.com>
---
arch/arm/configs/davinci_all_defconfig | 4 +---
1 file changed, 1 insertion(+), 3 deletions(-)
diff --git a/arch/arm/configs/davinci_all_defconfig b/arch/arm/configs/davinci_all_defconfig
index c8663ea..93aab3d 100644
--- a/arch/arm/configs/davinci_all_defconfig
+++ b/arch/arm/configs/davinci_all_defconfig
@@ -74,12 +74,10 @@ CONFIG_BLK_DEV_RAM=y
CONFIG_BLK_DEV_RAM_COUNT=1
CONFIG_BLK_DEV_RAM_SIZE=32768
CONFIG_EEPROM_AT24=y
-CONFIG_IDE=m
-CONFIG_BLK_DEV_PALMCHIP_BK3710=m
-CONFIG_SCSI=m
CONFIG_BLK_DEV_SD=m
CONFIG_ATA=m
CONFIG_AHCI_DA850=m
+CONFIG_PATA_BK3710=m
CONFIG_NETDEVICES=y
CONFIG_NETCONSOLE=y
CONFIG_TUN=m
--
1.9.1
^ permalink raw reply related
* [PATCH v3 1/5] ata: add Palmchip BK3710 PATA controller driver
From: Bartlomiej Zolnierkiewicz @ 2017-03-22 18:20 UTC (permalink / raw)
To: Tejun Heo
Cc: Sekhar Nori, Sergei Shtylyov, Kevin Hilman, Arnd Bergmann,
Russell King, Dmitry Eremin-Solenikov, linux-ide,
linux-arm-kernel, linux-kernel, b.zolnierkie
In-Reply-To: <1490206862-26065-1-git-send-email-b.zolnierkie@samsung.com>
Add Palmchip BK3710 PATA controller driver.
Signed-off-by: Bartlomiej Zolnierkiewicz <b.zolnierkie@samsung.com>
---
drivers/ata/Kconfig | 9 ++
drivers/ata/Makefile | 1 +
drivers/ata/pata_bk3710.c | 382 ++++++++++++++++++++++++++++++++++++++++++++++
3 files changed, 392 insertions(+)
create mode 100644 drivers/ata/pata_bk3710.c
diff --git a/drivers/ata/Kconfig b/drivers/ata/Kconfig
index 70b57d2..38fa4ac 100644
--- a/drivers/ata/Kconfig
+++ b/drivers/ata/Kconfig
@@ -510,6 +510,15 @@ config PATA_BF54X
If unsure, say N.
+config PATA_BK3710
+ tristate "Palmchip BK3710 PATA support"
+ depends on ARCH_DAVINCI
+ help
+ This option enables support for the integrated IDE controller on
+ the TI DaVinci SoC.
+
+ If unsure, say N.
+
config PATA_CMD64X
tristate "CMD64x PATA support"
depends on PCI
diff --git a/drivers/ata/Makefile b/drivers/ata/Makefile
index 89a0a19..9438db8 100644
--- a/drivers/ata/Makefile
+++ b/drivers/ata/Makefile
@@ -50,6 +50,7 @@ obj-$(CONFIG_PATA_ARTOP) += pata_artop.o
obj-$(CONFIG_PATA_ATIIXP) += pata_atiixp.o
obj-$(CONFIG_PATA_ATP867X) += pata_atp867x.o
obj-$(CONFIG_PATA_BF54X) += pata_bf54x.o
+obj-$(CONFIG_PATA_BK3710) += pata_bk3710.o
obj-$(CONFIG_PATA_CMD64X) += pata_cmd64x.o
obj-$(CONFIG_PATA_CS5520) += pata_cs5520.o
obj-$(CONFIG_PATA_CS5530) += pata_cs5530.o
diff --git a/drivers/ata/pata_bk3710.c b/drivers/ata/pata_bk3710.c
new file mode 100644
index 0000000..1a1223f
--- /dev/null
+++ b/drivers/ata/pata_bk3710.c
@@ -0,0 +1,382 @@
+/*
+ * Palmchip BK3710 PATA controller driver
+ *
+ * Copyright (c) 2017 Samsung Electronics Co., Ltd.
+ * http://www.samsung.com
+ *
+ * Based on palm_bk3710.c:
+ *
+ * Copyright (C) 2006 Texas Instruments.
+ * Copyright (C) 2007 MontaVista Software, Inc., <source@mvista.com>
+ *
+ * This file is subject to the terms and conditions of the GNU General Public
+ * License. See the file "COPYING" in the main directory of this archive
+ * for more details.
+ */
+
+#include <linux/ata.h>
+#include <linux/clk.h>
+#include <linux/delay.h>
+#include <linux/init.h>
+#include <linux/ioport.h>
+#include <linux/kernel.h>
+#include <linux/libata.h>
+#include <linux/module.h>
+#include <linux/platform_device.h>
+#include <linux/types.h>
+
+#define DRV_NAME "pata_bk3710"
+
+#define BK3710_TF_OFFSET 0x1F0
+#define BK3710_CTL_OFFSET 0x3F6
+
+#define BK3710_BMISP 0x02
+#define BK3710_IDETIMP 0x40
+#define BK3710_UDMACTL 0x48
+#define BK3710_MISCCTL 0x50
+#define BK3710_REGSTB 0x54
+#define BK3710_REGRCVR 0x58
+#define BK3710_DATSTB 0x5C
+#define BK3710_DATRCVR 0x60
+#define BK3710_DMASTB 0x64
+#define BK3710_DMARCVR 0x68
+#define BK3710_UDMASTB 0x6C
+#define BK3710_UDMATRP 0x70
+#define BK3710_UDMAENV 0x74
+#define BK3710_IORDYTMP 0x78
+
+static struct scsi_host_template pata_bk3710_sht = {
+ ATA_BMDMA_SHT(DRV_NAME),
+};
+
+static unsigned int ideclk_period; /* in nanoseconds */
+
+struct pata_bk3710_udmatiming {
+ unsigned int rptime; /* tRP -- Ready to pause time (nsec) */
+ unsigned int cycletime; /* tCYCTYP2/2 -- avg Cycle Time (nsec) */
+ /* tENV is always a minimum of 20 nsec */
+};
+
+static const struct pata_bk3710_udmatiming pata_bk3710_udmatimings[6] = {
+ { 160, 240 / 2 }, /* UDMA Mode 0 */
+ { 125, 160 / 2 }, /* UDMA Mode 1 */
+ { 100, 120 / 2 }, /* UDMA Mode 2 */
+ { 100, 90 / 2 }, /* UDMA Mode 3 */
+ { 100, 60 / 2 }, /* UDMA Mode 4 */
+ { 85, 40 / 2 }, /* UDMA Mode 5 */
+};
+
+static void pata_bk3710_setudmamode(void __iomem *base, unsigned int dev,
+ unsigned int mode)
+{
+ u32 val32;
+ u16 val16;
+ u8 tenv, trp, t0;
+
+ /* DMA Data Setup */
+ t0 = DIV_ROUND_UP(pata_bk3710_udmatimings[mode].cycletime,
+ ideclk_period) - 1;
+ tenv = DIV_ROUND_UP(20, ideclk_period) - 1;
+ trp = DIV_ROUND_UP(pata_bk3710_udmatimings[mode].rptime,
+ ideclk_period) - 1;
+
+ /* udmastb Ultra DMA Access Strobe Width */
+ val32 = ioread32(base + BK3710_UDMASTB) & (0xFF << (dev ? 0 : 8));
+ val32 |= t0 << (dev ? 8 : 0);
+ iowrite32(val32, base + BK3710_UDMASTB);
+
+ /* udmatrp Ultra DMA Ready to Pause Time */
+ val32 = ioread32(base + BK3710_UDMATRP) & (0xFF << (dev ? 0 : 8));
+ val32 |= trp << (dev ? 8 : 0);
+ iowrite32(val32, base + BK3710_UDMATRP);
+
+ /* udmaenv Ultra DMA envelop Time */
+ val32 = ioread32(base + BK3710_UDMAENV) & (0xFF << (dev ? 0 : 8));
+ val32 |= tenv << (dev ? 8 : 0);
+ iowrite32(val32, base + BK3710_UDMAENV);
+
+ /* Enable UDMA for Device */
+ val16 = ioread16(base + BK3710_UDMACTL) | (1 << dev);
+ iowrite16(val16, base + BK3710_UDMACTL);
+}
+
+static void pata_bk3710_setmwdmamode(void __iomem *base, unsigned int dev,
+ unsigned short min_cycle,
+ unsigned int mode)
+{
+ const struct ata_timing *t;
+ int cycletime;
+ u32 val32;
+ u16 val16;
+ u8 td, tkw, t0;
+
+ t = ata_timing_find_mode(mode);
+ cycletime = max_t(int, t->cycle, min_cycle);
+
+ /* DMA Data Setup */
+ t0 = DIV_ROUND_UP(cycletime, ideclk_period);
+ td = DIV_ROUND_UP(t->active, ideclk_period);
+ tkw = t0 - td - 1;
+ td--;
+
+ val32 = ioread32(base + BK3710_DMASTB) & (0xFF << (dev ? 0 : 8));
+ val32 |= td << (dev ? 8 : 0);
+ iowrite32(val32, base + BK3710_DMASTB);
+
+ val32 = ioread32(base + BK3710_DMARCVR) & (0xFF << (dev ? 0 : 8));
+ val32 |= tkw << (dev ? 8 : 0);
+ iowrite32(val32, base + BK3710_DMARCVR);
+
+ /* Disable UDMA for Device */
+ val16 = ioread16(base + BK3710_UDMACTL) & ~(1 << dev);
+ iowrite16(val16, base + BK3710_UDMACTL);
+}
+
+static void pata_bk3710_set_dmamode(struct ata_port *ap,
+ struct ata_device *adev)
+{
+ void __iomem *base = (void __iomem *)ap->ioaddr.bmdma_addr;
+ int is_slave = adev->devno;
+ const u8 xferspeed = adev->dma_mode;
+
+ if (xferspeed >= XFER_UDMA_0)
+ pata_bk3710_setudmamode(base, is_slave,
+ xferspeed - XFER_UDMA_0);
+ else
+ pata_bk3710_setmwdmamode(base, is_slave,
+ adev->id[ATA_ID_EIDE_DMA_MIN],
+ xferspeed);
+}
+
+static void pata_bk3710_setpiomode(void __iomem *base, struct ata_device *pair,
+ unsigned int dev, unsigned int cycletime,
+ unsigned int mode)
+{
+ const struct ata_timing *t;
+ u32 val32;
+ u8 t2, t2i, t0;
+
+ t = ata_timing_find_mode(XFER_PIO_0 + mode);
+
+ /* PIO Data Setup */
+ t0 = DIV_ROUND_UP(cycletime, ideclk_period);
+ t2 = DIV_ROUND_UP(t->active, ideclk_period);
+
+ t2i = t0 - t2 - 1;
+ t2--;
+
+ val32 = ioread32(base + BK3710_DATSTB) & (0xFF << (dev ? 0 : 8));
+ val32 |= t2 << (dev ? 8 : 0);
+ iowrite32(val32, base + BK3710_DATSTB);
+
+ val32 = ioread32(base + BK3710_DATRCVR) & (0xFF << (dev ? 0 : 8));
+ val32 |= t2i << (dev ? 8 : 0);
+ iowrite32(val32, base + BK3710_DATRCVR);
+
+ /* FIXME: this is broken also in the old driver */
+ if (pair) {
+ u8 mode2 = pair->pio_mode - XFER_PIO_0;
+
+ if (mode2 < mode)
+ mode = mode2;
+ }
+
+ /* TASKFILE Setup */
+ t0 = DIV_ROUND_UP(t->cyc8b, ideclk_period);
+ t2 = DIV_ROUND_UP(t->act8b, ideclk_period);
+
+ t2i = t0 - t2 - 1;
+ t2--;
+
+ val32 = ioread32(base + BK3710_REGSTB) & (0xFF << (dev ? 0 : 8));
+ val32 |= t2 << (dev ? 8 : 0);
+ iowrite32(val32, base + BK3710_REGSTB);
+
+ val32 = ioread32(base + BK3710_REGRCVR) & (0xFF << (dev ? 0 : 8));
+ val32 |= t2i << (dev ? 8 : 0);
+ iowrite32(val32, base + BK3710_REGRCVR);
+}
+
+static void pata_bk3710_set_piomode(struct ata_port *ap,
+ struct ata_device *adev)
+{
+ void __iomem *base = (void __iomem *)ap->ioaddr.bmdma_addr;
+ struct ata_device *pair = ata_dev_pair(adev);
+ const struct ata_timing *t = ata_timing_find_mode(adev->pio_mode);
+ const u16 *id = adev->id;
+ unsigned int cycle_time = 0;
+ int is_slave = adev->devno;
+ const u8 pio = adev->pio_mode - XFER_PIO_0;
+
+ if (id[ATA_ID_FIELD_VALID] & 2) {
+ if (ata_id_has_iordy(id))
+ cycle_time = id[ATA_ID_EIDE_PIO_IORDY];
+ else
+ cycle_time = id[ATA_ID_EIDE_PIO];
+
+ /* conservative "downgrade" for all pre-ATA2 drives */
+ if (pio < 3 && cycle_time < t->cycle)
+ cycle_time = 0; /* use standard timing */
+ }
+
+ if (!cycle_time)
+ cycle_time = t->cycle;
+
+ pata_bk3710_setpiomode(base, pair, is_slave, cycle_time, pio);
+}
+
+static void pata_bk3710_chipinit(void __iomem *base)
+{
+ /*
+ * REVISIT: the ATA reset signal needs to be managed through a
+ * GPIO, which means it should come from platform_data. Until
+ * we get and use such information, we have to trust that things
+ * have been reset before we get here.
+ */
+
+ /*
+ * Program the IDETIMP Register Value based on the following assumptions
+ *
+ * (ATA_IDETIMP_IDEEN , ENABLE ) |
+ * (ATA_IDETIMP_PREPOST1 , DISABLE) |
+ * (ATA_IDETIMP_PREPOST0 , DISABLE) |
+ *
+ * DM6446 silicon rev 2.1 and earlier have no observed net benefit
+ * from enabling prefetch/postwrite.
+ */
+ iowrite16(BIT(15), base + BK3710_IDETIMP);
+
+ /*
+ * UDMACTL Ultra-ATA DMA Control
+ * (ATA_UDMACTL_UDMAP1 , 0 ) |
+ * (ATA_UDMACTL_UDMAP0 , 0 )
+ *
+ */
+ iowrite16(0, base + BK3710_UDMACTL);
+
+ /*
+ * MISCCTL Miscellaneous Conrol Register
+ * (ATA_MISCCTL_HWNHLD1P , 1 cycle)
+ * (ATA_MISCCTL_HWNHLD0P , 1 cycle)
+ * (ATA_MISCCTL_TIMORIDE , 1)
+ */
+ iowrite32(0x001, base + BK3710_MISCCTL);
+
+ /*
+ * IORDYTMP IORDY Timer for Primary Register
+ * (ATA_IORDYTMP_IORDYTMP , 0xffff )
+ */
+ iowrite32(0xFFFF, base + BK3710_IORDYTMP);
+
+ /*
+ * Configure BMISP Register
+ * (ATA_BMISP_DMAEN1 , DISABLE ) |
+ * (ATA_BMISP_DMAEN0 , DISABLE ) |
+ * (ATA_BMISP_IORDYINT , CLEAR) |
+ * (ATA_BMISP_INTRSTAT , CLEAR) |
+ * (ATA_BMISP_DMAERROR , CLEAR)
+ */
+ iowrite16(0, base + BK3710_BMISP);
+
+ pata_bk3710_setpiomode(base, NULL, 0, 600, 0);
+ pata_bk3710_setpiomode(base, NULL, 1, 600, 0);
+}
+
+static struct ata_port_operations pata_bk3710_ports_ops = {
+ .inherits = &ata_bmdma_port_ops,
+ .cable_detect = ata_cable_80wire,
+
+ .set_piomode = pata_bk3710_set_piomode,
+ .set_dmamode = pata_bk3710_set_dmamode,
+};
+
+static int __init pata_bk3710_probe(struct platform_device *pdev)
+{
+ struct clk *clk;
+ struct resource *mem;
+ struct ata_host *host;
+ struct ata_port *ap;
+ void __iomem *base;
+ unsigned long rate;
+ int irq;
+
+ clk = devm_clk_get(&pdev->dev, NULL);
+ if (IS_ERR(clk))
+ return -ENODEV;
+
+ clk_enable(clk);
+ rate = clk_get_rate(clk);
+ if (!rate)
+ return -EINVAL;
+
+ /* NOTE: round *down* to meet minimum timings; we count in clocks */
+ ideclk_period = 1000000000UL / rate;
+
+ mem = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+
+ irq = platform_get_irq(pdev, 0);
+ if (irq < 0) {
+ pr_err(DRV_NAME ": failed to get IRQ resource\n");
+ return irq;
+ }
+
+ base = devm_ioremap_resource(&pdev->dev, mem);
+ if (IS_ERR(base))
+ return PTR_ERR(base);
+
+ /* configure the Palmchip controller */
+ pata_bk3710_chipinit(base);
+
+ /* allocate host */
+ host = ata_host_alloc(&pdev->dev, 1);
+ if (!host)
+ return -ENOMEM;
+ ap = host->ports[0];
+
+ ap->ops = &pata_bk3710_ports_ops;
+ ap->pio_mask = ATA_PIO4;
+ ap->mwdma_mask = ATA_MWDMA2;
+ ap->udma_mask = rate < 100000000 ? ATA_UDMA4 : ATA_UDMA5;
+ ap->flags |= ATA_FLAG_SLAVE_POSS;
+
+ ap->ioaddr.data_addr = base + BK3710_TF_OFFSET;
+ ap->ioaddr.error_addr = base + BK3710_TF_OFFSET + 1;
+ ap->ioaddr.feature_addr = base + BK3710_TF_OFFSET + 1;
+ ap->ioaddr.nsect_addr = base + BK3710_TF_OFFSET + 2;
+ ap->ioaddr.lbal_addr = base + BK3710_TF_OFFSET + 3;
+ ap->ioaddr.lbam_addr = base + BK3710_TF_OFFSET + 4;
+ ap->ioaddr.lbah_addr = base + BK3710_TF_OFFSET + 5;
+ ap->ioaddr.device_addr = base + BK3710_TF_OFFSET + 6;
+ ap->ioaddr.status_addr = base + BK3710_TF_OFFSET + 7;
+ ap->ioaddr.command_addr = base + BK3710_TF_OFFSET + 7;
+
+ ap->ioaddr.altstatus_addr = base + BK3710_CTL_OFFSET;
+ ap->ioaddr.ctl_addr = base + BK3710_CTL_OFFSET;
+
+ ap->ioaddr.bmdma_addr = base;
+
+ ata_port_desc(ap, "cmd 0x%lx ctl 0x%lx",
+ (unsigned long)base + BK3710_TF_OFFSET,
+ (unsigned long)base + BK3710_CTL_OFFSET);
+
+ /* activate */
+ return ata_host_activate(host, irq, ata_sff_interrupt, 0,
+ &pata_bk3710_sht);
+}
+
+/* work with hotplug and coldplug */
+MODULE_ALIAS("platform:palm_bk3710");
+
+static struct platform_driver pata_bk3710_driver = {
+ .driver = {
+ .name = "palm_bk3710",
+ },
+};
+
+static int __init pata_bk3710_init(void)
+{
+ return platform_driver_probe(&pata_bk3710_driver, pata_bk3710_probe);
+}
+
+module_init(pata_bk3710_init);
+MODULE_LICENSE("GPL");
--
1.9.1
^ permalink raw reply related
* Re: support ranges TRIM for libata
From: Christoph Hellwig @ 2017-03-22 18:19 UTC (permalink / raw)
To: Tejun Heo
Cc: Christoph Hellwig, martin.petersen, axboe, linux-ide, linux-scsi,
linux-block
In-Reply-To: <20170321185901.GB3706@htj.duckdns.org>
On Tue, Mar 21, 2017 at 02:59:01PM -0400, Tejun Heo wrote:
> I do like the fact that this is a lot simpler than the previous
> implementation but am not quite sure we want to deviate significantly
> from what we do for other commands (command translation). Is it
> because fixing the existing implementation would involve invaisve
> changes including memory allocations?
The current implementation already has the issue of that it does
corrupt user data reliably if the using SG_IO for WRITE SAME commands.
Doing ranges using translation would turn into a nightmare because
ATA TRIM ranges are 16 bits long while SCSI UNAMP ranges are 32-bit,
so we effectively can't translated them without introducing a
non-standard hook between libata and scsi to communicate that
limit. And once we're down that path we might as well just do the
right thing directly.
^ permalink raw reply
* Re: [PATCH v2 1/3] ata: add Palmchip BK3710 PATA controller driver
From: Bartlomiej Zolnierkiewicz @ 2017-03-22 17:59 UTC (permalink / raw)
To: Sergei Shtylyov
Cc: Tejun Heo, Sekhar Nori, Kevin Hilman, Arnd Bergmann, Russell King,
Dmitry Eremin-Solenikov, linux-ide, linux-arm-kernel,
linux-kernel
In-Reply-To: <bc9a2d9b-1f16-4b59-0033-235686c76dcb@cogentembedded.com>
Hi,
On Saturday, March 18, 2017 04:52:18 PM Sergei Shtylyov wrote:
> Hello!
>
> On 3/14/2017 7:36 PM, Bartlomiej Zolnierkiewicz wrote:
>
> > Add Palmchip BK3710 PATA controller driver.
> >
> > Signed-off-by: Bartlomiej Zolnierkiewicz <b.zolnierkie@samsung.com>
> [...]
> > diff --git a/drivers/ata/pata_bk3710.c b/drivers/ata/pata_bk3710.c
> > new file mode 100644
> > index 0000000..6d77217
> > --- /dev/null
> > +++ b/drivers/ata/pata_bk3710.c
> > @@ -0,0 +1,386 @@
> [...]
> > +static void pata_bk3710_chipinit(void __iomem *base)
> > +{
> [...]
> > + /*
> > + * IORDYTMP IORDY Timer for Primary Register
> > + * (ATA_IORDYTMP_IORDYTMP , 0xffff )
> > + */
> > + iowrite32(0xFFFF, base + BK3710_IORDYTMP);
>
> As I've already said, this is useless as we don't handle the IORDY timeout
> interrupt anyway; writing 0 would be fine.
Will fix in v3, in the incremental patch (so it is easier to revert
if it turns out to cause problems later or port to palm_bk3710).
> > +
> > + /*
> > + * Configure BMISP Register
> > + * (ATA_BMISP_DMAEN1 , DISABLE ) |
> > + * (ATA_BMISP_DMAEN0 , DISABLE ) |
> > + * (ATA_BMISP_IORDYINT , CLEAR) |
> > + * (ATA_BMISP_INTRSTAT , CLEAR) |
> > + * (ATA_BMISP_DMAERROR , CLEAR)
> > + */
> > + iowrite16(0, base + BK3710_BMISP);
>
> Bits 0-3 cane only be cleared by writing 1, so this write can't clear
The documentation does say this about bits 1-3, bit 0 is handled in
a different way.
> them, contrary to what the comment says. Might be a material for a follow-up
> patch tho...
Will fix in the incremental patch in v3.
> [...]
> > +static int __init pata_bk3710_probe(struct platform_device *pdev)
> > +{
> > + struct clk *clk;
> > + struct resource *mem;
> > + struct ata_host *host;
> > + struct ata_port *ap;
> > + void __iomem *base;
> > + unsigned long rate;
> > + int irq;
> > +
> > + clk = devm_clk_get(&pdev->dev, NULL);
> > + if (IS_ERR(clk))
> > + return -ENODEV;
> > +
> > + clk_enable(clk);
> > + rate = clk_get_rate(clk);
> > + if (!rate)
> > + return -EINVAL;
> > +
> > + /* NOTE: round *down* to meet minimum timings; we count in clocks */
> > + ideclk_period = 1000000000UL / rate;
> > +
> > + mem = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> > + if (mem == NULL) {
> > + pr_err(DRV_NAME ": failed to get memory region resource\n");
> > + return -ENODEV;
> > + }
>
> NULL check not needed here, devm_ioremap_resource() checks this anyway.
Will be fixed in v3.
> > +
> > + irq = platform_get_irq(pdev, 0);
> > + if (irq < 0) {
> > + pr_err(DRV_NAME ": failed to get IRQ resource\n");
> > + return irq;
> > + }
> > +
> > + base = devm_ioremap_resource(&pdev->dev, mem);
> > + if (IS_ERR(base))
> > + return PTR_ERR(base);
> > +
> [...]
> > +/* work with hotplug and coldplug */
> > +MODULE_ALIAS("platform:palm_bk3710");
> > +
> > +static struct platform_driver pata_bk3710_driver = {
> > + .driver = {
> > + .name = "palm_bk3710",
>
> Not DRV_NAME?
DRV_NAME is "pata_bk3710" and the platform driver name needs to
match the old driver name for compatibility reasons (supporting
both drivers by the arch specific code).
Best regards,
--
Bartlomiej Zolnierkiewicz
Samsung R&D Institute Poland
Samsung Electronics
^ permalink raw reply
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