Messages in this thread | | | Subject | Re: [PATCH] ahci: added a new driver for supporting Freescale AHCI sata | From | Hans de Goede <> | Date | Mon, 7 Sep 2015 09:06:43 +0200 |
| |
Hi,
On 06-09-15 07:39, Yuantian Tang wrote: > Hi, > >> -----Original Message----- >> From: Hans de Goede [mailto:hdegoede@redhat.com] >> Sent: Wednesday, September 02, 2015 4:32 PM >> To: Tang Yuantian-B29983 <Yuantian.Tang@freescale.com> >> Cc: tj@kernel.org; linux-ide@vger.kernel.org; linux-kernel@vger.kernel.org >> Subject: Re: [PATCH] ahci: added a new driver for supporting Freescale AHCI >> sata >> >> Hi, >> >> On 02-09-15 04:25, Yuantian.Tang@freescale.com wrote: >>> From: Tang Yuantian <Yuantian.Tang@freescale.com> >>> >>> Currently Freescale QorIQ series SATA is supported by ahci_platform >>> driver. Some SoC specific settings have been put in uboot. So whether >>> SATA works or not heavily depends on uboot. >>> This patch will add a new driver to support QorIQ sata which removes >>> the dependency on any other boot loader. >>> Freescale QorIQ series sata, like ls1021a ls2085a ls1043a, is >>> compatible with serial ATA 3.0 and AHCI 1.3 specification. >>> >>> Signed-off-by: Yuantian Tang <Yuantian.Tang@freescale.com> >> >> Thanks for the patch looks good overall. >> >> You need to add a Documentation/devicetree/bindings/ata/ahci-fsl-qoriq.txt >> (or a similar named file) documenting the compatible strings and what the >> devicetree nodes should contain wrt reg, interrupts, etc. >> properties. See Documentation/devicetree/bindings/ata/ahci-platform.txt >> as an example. >> >> Further comments inline. >> > I was about to use ahci_platform driver, so I added the bindings stuff to > Documentation/devicetree/bindings/ata/ahci-platform.txt > So I need to revert the old bingings first and then add a new one. > >>> --- >>> drivers/ata/Kconfig | 9 ++ >>> drivers/ata/Makefile | 1 + >>> drivers/ata/ahci_platform.c | 1 - >>> drivers/ata/ahci_qoriq.c | 308 >> ++++++++++++++++++++++++++++++++++++++++++++ >>> 4 files changed, 318 insertions(+), 1 deletion(-) >>> create mode 100644 drivers/ata/ahci_qoriq.c >>> >>> diff --git a/drivers/ata/Kconfig b/drivers/ata/Kconfig index >>> 15e40ee..6aaa3f8 100644 >>> --- a/drivers/ata/Kconfig >>> +++ b/drivers/ata/Kconfig >>> @@ -175,6 +175,15 @@ config AHCI_XGENE >>> help >>> This option enables support for APM X-Gene SoC SATA host >> controller. >>> >>> +config AHCI_QORIQ >>> + tristate "Freescale QorIQ AHCI SATA support" >>> + depends on OF >>> + help >>> + This option enables support for the Freescale QorIQ AHCI SoC's >>> + onboard AHCI SATA. >>> + >>> + If unsure, say N. >>> + >>> config SATA_FSL >>> tristate "Freescale 3.0Gbps SATA support" >>> depends on FSL_SOC >>> diff --git a/drivers/ata/Makefile b/drivers/ata/Makefile index >>> af70919..af45eff 100644 >>> --- a/drivers/ata/Makefile >>> +++ b/drivers/ata/Makefile >>> @@ -19,6 +19,7 @@ obj-$(CONFIG_AHCI_SUNXI) += ahci_sunxi.o >> libahci.o libahci_platform.o >>> obj-$(CONFIG_AHCI_ST) += ahci_st.o libahci.o >> libahci_platform.o >>> obj-$(CONFIG_AHCI_TEGRA) += ahci_tegra.o libahci.o >> libahci_platform.o >>> obj-$(CONFIG_AHCI_XGENE) += ahci_xgene.o libahci.o >> libahci_platform.o >>> +obj-$(CONFIG_AHCI_QORIQ) += ahci_qoriq.o libahci.o >> libahci_platform.o >>> >>> # SFF w/ custom DMA >>> obj-$(CONFIG_PDC_ADMA) += pdc_adma.o >>> diff --git a/drivers/ata/ahci_platform.c b/drivers/ata/ahci_platform.c >>> index 1befb11..04975b8 100644 >>> --- a/drivers/ata/ahci_platform.c >>> +++ b/drivers/ata/ahci_platform.c >>> @@ -76,7 +76,6 @@ static const struct of_device_id ahci_of_match[] = { >>> { .compatible = "ibm,476gtr-ahci", }, >>> { .compatible = "snps,dwc-ahci", }, >>> { .compatible = "hisilicon,hisi-ahci", }, >>> - { .compatible = "fsl,qoriq-ahci", }, >>> {}, >>> }; >>> MODULE_DEVICE_TABLE(of, ahci_of_match); >> >> This will break booting new kernels with old dtb files, something which in >> general is considered a big non-no, I suggest adding a comment that this has >> been superseded by the new ahci_qoriq.c code, and maybe add a date to >> retire the compatible in say a year from now. >> > There is no old dtb because LS* platforms are not been upstreamed yet. > So I think we can safely replace it without breaking anything.
Ok / ACK.
Regards,
Hans
| |