| Message ID | 20201109173948.96663-1-sven.auhagen@voleatech.de (mailing list archive) |
|---|---|
| Headers | show |
| Series | Armada8k enable per-port SATA interrupts and drop a hack in the IRQ subsystem | expand |
Hello, can I ask about the status of this patch? As far as I can tell it was not merged to ata and I did not receive any further feedback that there was a problem with the patch series. As a matter of fact the device tree part was already merged by Gregory Clement. Best and thanks Sven On Mon, Nov 09, 2020 at 06:39:39PM +0100, sven.auhagen@voleatech.de wrote: > From: Sven Auhagen <sven.auhagen@voleatech.de> > > Hello, > > There were already 4 versions of this series from Miquèl. > I talked to Miquèl and I fixed up the last comments from v4. > I am looking for feedback if this patch series is now ready to be merged > and what should be further changed. > > Here is the original cover letter: > > Some time ago, when the initial support for Armada CP110 was > contributed, the SATA core was not able to handle per-port > interrupts. Despite the hardware reality, the device tree only > represents one main interrupt for the two ports. Having both SATA > ports enabled at the same time has been achieved by a hack in the ICU > driver(1) that faked the use of the two interrupts, no matter which > SATA port was in use. > > Now that the SATA core is ready to handle more than one interrupt, > this series adds support for it in the libahci_platform code. The > CP110 device tree must be updated to reflect the two SATA ports > available and their respective interrupts. To do not break DT backward > compatibility, the ahci_platform driver now embeds a special quirk > which checks if the DT is valid (only for A8k compatible) and, if > needed, creates the two missing sub-nodes, and assign them the > relevant "reg" and "interrupts" properties, before removing the main > SATA node "interrupts" one. > > (1) The ICU is an irqchip aggregating the CP110 (south-bridge) > interrupts into MSIs for the AP806 (north-bridge). > > Best > Sven > > Change from v2: > * Fix commit message of custom irq init for host init > > Change from v1: > * Add a patch to enable custom irq initialization in > plattform init host > * Add multi_irq_host_ack callback for the msi irq handler > * Rework the ahci mvebu patch to initiate the irq and use > the new multi_irq_host_ack to handle the custom irq code. > Remove the custom irq handler and duplicate code. > * Fix the armada8k backwards compatibility code > * Rename AHCI_PLATFORM_A8K_QUIRK to AHCI_PLATFORM_ARMADA8K_QUIRK > > Miquel Raynal (5): > ata: ahci: mvebu: Rename a platform data flag > ata: ahci: mvebu: Support A8k compatible > irqchip/irq-mvebu-icu: Remove the double SATA ports interrupt hack > dt-bindings: ata: Update ahci bindings with possible per-port > interrupts > dt-bindings: ata: Update ahci_mvebu bindings > > Sven Auhagen (4): > ata: libahci_platform: Do not try to get an IRQ when > AHCI_HFLAG_MULTI_MSI is set > ata: ahci: add ack callback to multi irq handler > ata: ahci: mvebu: Add support for A8k legacy DT bindings > arm64: dts: marvell: armada-cp110: Switch to per-port SATA interrupts > > .../devicetree/bindings/ata/ahci-platform.txt | 7 + > arch/arm64/boot/dts/marvell/armada-cp11x.dtsi | 6 +- > drivers/ata/ahci.h | 2 + > drivers/ata/ahci_mvebu.c | 143 ++++++++++++++++-- > drivers/ata/libahci.c | 4 + > drivers/ata/libahci_platform.c | 19 ++- > drivers/irqchip/irq-mvebu-icu.c | 18 --- > include/linux/ahci_platform.h | 1 + > 8 files changed, 160 insertions(+), 40 deletions(-) > > -- > 2.20.1 > >
[Resend in plain text] Hi, Just letting everyone know - merging only the DT part of this patchset broke AHCI on all Marvell Armada 7k8k / CN913x platforms in v5.11 release. FYI, I'm currently updating the device trees in EDK2 based on the v5.11 DT sources - I'm going to keep the previous binding anyway, as the new one breaks booting with AHCI not only for older Linux versions, but also for other OSs (yet another argument to use ACPI). Best regards, Marcin sob., 27 lut 2021 o 12:02 Sven Auhagen <sven.auhagen@voleatech.de> napisał(a): > > Hello, > > can I ask about the status of this patch? > As far as I can tell it was not merged to ata and I did not receive > any further feedback that there was a problem with the patch series. > > As a matter of fact the device tree part was already merged by > Gregory Clement. > > Best and thanks > Sven > > On Mon, Nov 09, 2020 at 06:39:39PM +0100, sven.auhagen@voleatech.de wrote: > > From: Sven Auhagen <sven.auhagen@voleatech.de> > > > > Hello, > > > > There were already 4 versions of this series from Miquèl. > > I talked to Miquèl and I fixed up the last comments from v4. > > I am looking for feedback if this patch series is now ready to be merged > > and what should be further changed. > > > > Here is the original cover letter: > > > > Some time ago, when the initial support for Armada CP110 was > > contributed, the SATA core was not able to handle per-port > > interrupts. Despite the hardware reality, the device tree only > > represents one main interrupt for the two ports. Having both SATA > > ports enabled at the same time has been achieved by a hack in the ICU > > driver(1) that faked the use of the two interrupts, no matter which > > SATA port was in use. > > > > Now that the SATA core is ready to handle more than one interrupt, > > this series adds support for it in the libahci_platform code. The > > CP110 device tree must be updated to reflect the two SATA ports > > available and their respective interrupts. To do not break DT backward > > compatibility, the ahci_platform driver now embeds a special quirk > > which checks if the DT is valid (only for A8k compatible) and, if > > needed, creates the two missing sub-nodes, and assign them the > > relevant "reg" and "interrupts" properties, before removing the main > > SATA node "interrupts" one. > > > > (1) The ICU is an irqchip aggregating the CP110 (south-bridge) > > interrupts into MSIs for the AP806 (north-bridge). > > > > Best > > Sven > > > > Change from v2: > > * Fix commit message of custom irq init for host init > > > > Change from v1: > > * Add a patch to enable custom irq initialization in > > plattform init host > > * Add multi_irq_host_ack callback for the msi irq handler > > * Rework the ahci mvebu patch to initiate the irq and use > > the new multi_irq_host_ack to handle the custom irq code. > > Remove the custom irq handler and duplicate code. > > * Fix the armada8k backwards compatibility code > > * Rename AHCI_PLATFORM_A8K_QUIRK to AHCI_PLATFORM_ARMADA8K_QUIRK > > > > Miquel Raynal (5): > > ata: ahci: mvebu: Rename a platform data flag > > ata: ahci: mvebu: Support A8k compatible > > irqchip/irq-mvebu-icu: Remove the double SATA ports interrupt hack > > dt-bindings: ata: Update ahci bindings with possible per-port > > interrupts > > dt-bindings: ata: Update ahci_mvebu bindings > > > > Sven Auhagen (4): > > ata: libahci_platform: Do not try to get an IRQ when > > AHCI_HFLAG_MULTI_MSI is set > > ata: ahci: add ack callback to multi irq handler > > ata: ahci: mvebu: Add support for A8k legacy DT bindings > > arm64: dts: marvell: armada-cp110: Switch to per-port SATA interrupts > > > > .../devicetree/bindings/ata/ahci-platform.txt | 7 + > > arch/arm64/boot/dts/marvell/armada-cp11x.dtsi | 6 +- > > drivers/ata/ahci.h | 2 + > > drivers/ata/ahci_mvebu.c | 143 ++++++++++++++++-- > > drivers/ata/libahci.c | 4 + > > drivers/ata/libahci_platform.c | 19 ++- > > drivers/irqchip/irq-mvebu-icu.c | 18 --- > > include/linux/ahci_platform.h | 1 + > > 8 files changed, 160 insertions(+), 40 deletions(-) > > > > -- > > 2.20.1 > > > > > > _______________________________________________ > linux-arm-kernel mailing list > linux-arm-kernel@lists.infradead.org > http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
Hello Sven, > Hello, > > can I ask about the status of this patch? > As far as I can tell it was not merged to ata and I did not receive > any further feedback that there was a problem with the patch series. > > As a matter of fact the device tree part was already merged by > Gregory Clement. As the maintainer of the ahci subsytem reviewed the series, I really expected that he merged it, so me neither I don't understand what happened. Hans, is there still anything wrong that prevent you applying the series ? Grégory > > Best and thanks > Sven > > On Mon, Nov 09, 2020 at 06:39:39PM +0100, sven.auhagen@voleatech.de wrote: >> From: Sven Auhagen <sven.auhagen@voleatech.de> >> >> Hello, >> >> There were already 4 versions of this series from Miquèl. >> I talked to Miquèl and I fixed up the last comments from v4. >> I am looking for feedback if this patch series is now ready to be merged >> and what should be further changed. >> >> Here is the original cover letter: >> >> Some time ago, when the initial support for Armada CP110 was >> contributed, the SATA core was not able to handle per-port >> interrupts. Despite the hardware reality, the device tree only >> represents one main interrupt for the two ports. Having both SATA >> ports enabled at the same time has been achieved by a hack in the ICU >> driver(1) that faked the use of the two interrupts, no matter which >> SATA port was in use. >> >> Now that the SATA core is ready to handle more than one interrupt, >> this series adds support for it in the libahci_platform code. The >> CP110 device tree must be updated to reflect the two SATA ports >> available and their respective interrupts. To do not break DT backward >> compatibility, the ahci_platform driver now embeds a special quirk >> which checks if the DT is valid (only for A8k compatible) and, if >> needed, creates the two missing sub-nodes, and assign them the >> relevant "reg" and "interrupts" properties, before removing the main >> SATA node "interrupts" one. >> >> (1) The ICU is an irqchip aggregating the CP110 (south-bridge) >> interrupts into MSIs for the AP806 (north-bridge). >> >> Best >> Sven >> >> Change from v2: >> * Fix commit message of custom irq init for host init >> >> Change from v1: >> * Add a patch to enable custom irq initialization in >> plattform init host >> * Add multi_irq_host_ack callback for the msi irq handler >> * Rework the ahci mvebu patch to initiate the irq and use >> the new multi_irq_host_ack to handle the custom irq code. >> Remove the custom irq handler and duplicate code. >> * Fix the armada8k backwards compatibility code >> * Rename AHCI_PLATFORM_A8K_QUIRK to AHCI_PLATFORM_ARMADA8K_QUIRK >> >> Miquel Raynal (5): >> ata: ahci: mvebu: Rename a platform data flag >> ata: ahci: mvebu: Support A8k compatible >> irqchip/irq-mvebu-icu: Remove the double SATA ports interrupt hack >> dt-bindings: ata: Update ahci bindings with possible per-port >> interrupts >> dt-bindings: ata: Update ahci_mvebu bindings >> >> Sven Auhagen (4): >> ata: libahci_platform: Do not try to get an IRQ when >> AHCI_HFLAG_MULTI_MSI is set >> ata: ahci: add ack callback to multi irq handler >> ata: ahci: mvebu: Add support for A8k legacy DT bindings >> arm64: dts: marvell: armada-cp110: Switch to per-port SATA interrupts >> >> .../devicetree/bindings/ata/ahci-platform.txt | 7 + >> arch/arm64/boot/dts/marvell/armada-cp11x.dtsi | 6 +- >> drivers/ata/ahci.h | 2 + >> drivers/ata/ahci_mvebu.c | 143 ++++++++++++++++-- >> drivers/ata/libahci.c | 4 + >> drivers/ata/libahci_platform.c | 19 ++- >> drivers/irqchip/irq-mvebu-icu.c | 18 --- >> include/linux/ahci_platform.h | 1 + >> 8 files changed, 160 insertions(+), 40 deletions(-) >> >> -- >> 2.20.1 >> >>
Hello Marcin, > [Resend in plain text] > > Hi, > > Just letting everyone know - merging only the DT part of this patchset > broke AHCI on all Marvell Armada 7k8k / CN913x platforms in v5.11 > release. It's unfortunate that we didn't know this when v5.11-rc1 was released. However it is still time for a fix, I will submit it. As I explained in the other email when I applied this I really though that the driver part will be applied, I don't know what happened here. Grégory > > FYI, I'm currently updating the device trees in EDK2 based on the > v5.11 DT sources - I'm going to keep the previous binding anyway, as > the new one breaks booting with AHCI not only for older Linux > versions, but also for other OSs (yet another argument to use ACPI). > > Best regards, > Marcin > > > sob., 27 lut 2021 o 12:02 Sven Auhagen <sven.auhagen@voleatech.de> napisał(a): >> >> Hello, >> >> can I ask about the status of this patch? >> As far as I can tell it was not merged to ata and I did not receive >> any further feedback that there was a problem with the patch series. >> >> As a matter of fact the device tree part was already merged by >> Gregory Clement. >> >> Best and thanks >> Sven >> >> On Mon, Nov 09, 2020 at 06:39:39PM +0100, sven.auhagen@voleatech.de wrote: >> > From: Sven Auhagen <sven.auhagen@voleatech.de> >> > >> > Hello, >> > >> > There were already 4 versions of this series from Miquèl. >> > I talked to Miquèl and I fixed up the last comments from v4. >> > I am looking for feedback if this patch series is now ready to be merged >> > and what should be further changed. >> > >> > Here is the original cover letter: >> > >> > Some time ago, when the initial support for Armada CP110 was >> > contributed, the SATA core was not able to handle per-port >> > interrupts. Despite the hardware reality, the device tree only >> > represents one main interrupt for the two ports. Having both SATA >> > ports enabled at the same time has been achieved by a hack in the ICU >> > driver(1) that faked the use of the two interrupts, no matter which >> > SATA port was in use. >> > >> > Now that the SATA core is ready to handle more than one interrupt, >> > this series adds support for it in the libahci_platform code. The >> > CP110 device tree must be updated to reflect the two SATA ports >> > available and their respective interrupts. To do not break DT backward >> > compatibility, the ahci_platform driver now embeds a special quirk >> > which checks if the DT is valid (only for A8k compatible) and, if >> > needed, creates the two missing sub-nodes, and assign them the >> > relevant "reg" and "interrupts" properties, before removing the main >> > SATA node "interrupts" one. >> > >> > (1) The ICU is an irqchip aggregating the CP110 (south-bridge) >> > interrupts into MSIs for the AP806 (north-bridge). >> > >> > Best >> > Sven >> > >> > Change from v2: >> > * Fix commit message of custom irq init for host init >> > >> > Change from v1: >> > * Add a patch to enable custom irq initialization in >> > plattform init host >> > * Add multi_irq_host_ack callback for the msi irq handler >> > * Rework the ahci mvebu patch to initiate the irq and use >> > the new multi_irq_host_ack to handle the custom irq code. >> > Remove the custom irq handler and duplicate code. >> > * Fix the armada8k backwards compatibility code >> > * Rename AHCI_PLATFORM_A8K_QUIRK to AHCI_PLATFORM_ARMADA8K_QUIRK >> > >> > Miquel Raynal (5): >> > ata: ahci: mvebu: Rename a platform data flag >> > ata: ahci: mvebu: Support A8k compatible >> > irqchip/irq-mvebu-icu: Remove the double SATA ports interrupt hack >> > dt-bindings: ata: Update ahci bindings with possible per-port >> > interrupts >> > dt-bindings: ata: Update ahci_mvebu bindings >> > >> > Sven Auhagen (4): >> > ata: libahci_platform: Do not try to get an IRQ when >> > AHCI_HFLAG_MULTI_MSI is set >> > ata: ahci: add ack callback to multi irq handler >> > ata: ahci: mvebu: Add support for A8k legacy DT bindings >> > arm64: dts: marvell: armada-cp110: Switch to per-port SATA interrupts >> > >> > .../devicetree/bindings/ata/ahci-platform.txt | 7 + >> > arch/arm64/boot/dts/marvell/armada-cp11x.dtsi | 6 +- >> > drivers/ata/ahci.h | 2 + >> > drivers/ata/ahci_mvebu.c | 143 ++++++++++++++++-- >> > drivers/ata/libahci.c | 4 + >> > drivers/ata/libahci_platform.c | 19 ++- >> > drivers/irqchip/irq-mvebu-icu.c | 18 --- >> > include/linux/ahci_platform.h | 1 + >> > 8 files changed, 160 insertions(+), 40 deletions(-) >> > >> > -- >> > 2.20.1 >> > >> > >> >> _______________________________________________ >> linux-arm-kernel mailing list >> linux-arm-kernel@lists.infradead.org >> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
HI Gregory, pt., 19 mar 2021 o 08:35 Gregory CLEMENT <gregory.clement@bootlin.com> napisał(a): > > Hello Marcin, > > > [Resend in plain text] > > > > Hi, > > > > Just letting everyone know - merging only the DT part of this patchset > > broke AHCI on all Marvell Armada 7k8k / CN913x platforms in v5.11 > > release. > > It's unfortunate that we didn't know this when v5.11-rc1 was > released. However it is still time for a fix, I will submit it. > As I explained in the other email when I applied this I really though > that the driver part will be applied, I don't know what happened here. > Sure, looking at the thread it looks more of a communication issue. I am also surprised the breakage went unnoticed for a while (unless everyone is using edk2, like myself :) ). I think it would be good to revert the change on top of v5.11.x. The drivers adoption would have to land before v5.12 though, so that not to repeat the problem during next release. Small rant: A general issue with the DT binding changes of this kind (previously clocks, ICU, etc.) that I have, is a side effect of incompatibility with older kernels/other OSs. The latter must follow the modifications, but you can forget of booting e.g. Debian Buster with the ToT device tree. Therefore in edk2 I do not update the device tree fork to often and need to tweak it in order to have the widest support coverage. I'm dropping this note, so that in similar case in future, there is additional consideration, how the change impacts a wider ecosystem, not only today's Linux top of tree. Thanks, Marcin > > > > FYI, I'm currently updating the device trees in EDK2 based on the > > v5.11 DT sources - I'm going to keep the previous binding anyway, as > > the new one breaks booting with AHCI not only for older Linux > > versions, but also for other OSs (yet another argument to use ACPI). > > > > Best regards, > > Marcin > > > > > > sob., 27 lut 2021 o 12:02 Sven Auhagen <sven.auhagen@voleatech.de> napisał(a): > >> > >> Hello, > >> > >> can I ask about the status of this patch? > >> As far as I can tell it was not merged to ata and I did not receive > >> any further feedback that there was a problem with the patch series. > >> > >> As a matter of fact the device tree part was already merged by > >> Gregory Clement. > >> > >> Best and thanks > >> Sven > >> > >> On Mon, Nov 09, 2020 at 06:39:39PM +0100, sven.auhagen@voleatech.de wrote: > >> > From: Sven Auhagen <sven.auhagen@voleatech.de> > >> > > >> > Hello, > >> > > >> > There were already 4 versions of this series from Miquèl. > >> > I talked to Miquèl and I fixed up the last comments from v4. > >> > I am looking for feedback if this patch series is now ready to be merged > >> > and what should be further changed. > >> > > >> > Here is the original cover letter: > >> > > >> > Some time ago, when the initial support for Armada CP110 was > >> > contributed, the SATA core was not able to handle per-port > >> > interrupts. Despite the hardware reality, the device tree only > >> > represents one main interrupt for the two ports. Having both SATA > >> > ports enabled at the same time has been achieved by a hack in the ICU > >> > driver(1) that faked the use of the two interrupts, no matter which > >> > SATA port was in use. > >> > > >> > Now that the SATA core is ready to handle more than one interrupt, > >> > this series adds support for it in the libahci_platform code. The > >> > CP110 device tree must be updated to reflect the two SATA ports > >> > available and their respective interrupts. To do not break DT backward > >> > compatibility, the ahci_platform driver now embeds a special quirk > >> > which checks if the DT is valid (only for A8k compatible) and, if > >> > needed, creates the two missing sub-nodes, and assign them the > >> > relevant "reg" and "interrupts" properties, before removing the main > >> > SATA node "interrupts" one. > >> > > >> > (1) The ICU is an irqchip aggregating the CP110 (south-bridge) > >> > interrupts into MSIs for the AP806 (north-bridge). > >> > > >> > Best > >> > Sven > >> > > >> > Change from v2: > >> > * Fix commit message of custom irq init for host init > >> > > >> > Change from v1: > >> > * Add a patch to enable custom irq initialization in > >> > plattform init host > >> > * Add multi_irq_host_ack callback for the msi irq handler > >> > * Rework the ahci mvebu patch to initiate the irq and use > >> > the new multi_irq_host_ack to handle the custom irq code. > >> > Remove the custom irq handler and duplicate code. > >> > * Fix the armada8k backwards compatibility code > >> > * Rename AHCI_PLATFORM_A8K_QUIRK to AHCI_PLATFORM_ARMADA8K_QUIRK > >> > > >> > Miquel Raynal (5): > >> > ata: ahci: mvebu: Rename a platform data flag > >> > ata: ahci: mvebu: Support A8k compatible > >> > irqchip/irq-mvebu-icu: Remove the double SATA ports interrupt hack > >> > dt-bindings: ata: Update ahci bindings with possible per-port > >> > interrupts > >> > dt-bindings: ata: Update ahci_mvebu bindings > >> > > >> > Sven Auhagen (4): > >> > ata: libahci_platform: Do not try to get an IRQ when > >> > AHCI_HFLAG_MULTI_MSI is set > >> > ata: ahci: add ack callback to multi irq handler > >> > ata: ahci: mvebu: Add support for A8k legacy DT bindings > >> > arm64: dts: marvell: armada-cp110: Switch to per-port SATA interrupts > >> > > >> > .../devicetree/bindings/ata/ahci-platform.txt | 7 + > >> > arch/arm64/boot/dts/marvell/armada-cp11x.dtsi | 6 +- > >> > drivers/ata/ahci.h | 2 + > >> > drivers/ata/ahci_mvebu.c | 143 ++++++++++++++++-- > >> > drivers/ata/libahci.c | 4 + > >> > drivers/ata/libahci_platform.c | 19 ++- > >> > drivers/irqchip/irq-mvebu-icu.c | 18 --- > >> > include/linux/ahci_platform.h | 1 + > >> > 8 files changed, 160 insertions(+), 40 deletions(-) > >> > > >> > -- > >> > 2.20.1 > >> > > >> > > >> > >> _______________________________________________ > >> linux-arm-kernel mailing list > >> linux-arm-kernel@lists.infradead.org > >> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel > > -- > Gregory Clement, Bootlin > Embedded Linux and Kernel engineering > http://bootlin.com
On Fri, Mar 19, 2021 at 08:26:00AM +0100, Gregory CLEMENT wrote: Hello Grégory, > Hello Sven, > > > Hello, > > > > can I ask about the status of this patch? > > As far as I can tell it was not merged to ata and I did not receive > > any further feedback that there was a problem with the patch series. > > > > As a matter of fact the device tree part was already merged by > > Gregory Clement. > > As the maintainer of the ahci subsytem reviewed the series, I really > expected that he merged it, so me neither I don't understand what > happened. > I assumed the same and I did not get an answer to my last email regarding the status of the page. Best Sven > Hans, > > is there still anything wrong that prevent you applying the series ? > > Grégory > > > > > > Best and thanks > > Sven > > > > On Mon, Nov 09, 2020 at 06:39:39PM +0100, sven.auhagen@voleatech.de wrote: > >> From: Sven Auhagen <sven.auhagen@voleatech.de> > >> > >> Hello, > >> > >> There were already 4 versions of this series from Miquèl. > >> I talked to Miquèl and I fixed up the last comments from v4. > >> I am looking for feedback if this patch series is now ready to be merged > >> and what should be further changed. > >> > >> Here is the original cover letter: > >> > >> Some time ago, when the initial support for Armada CP110 was > >> contributed, the SATA core was not able to handle per-port > >> interrupts. Despite the hardware reality, the device tree only > >> represents one main interrupt for the two ports. Having both SATA > >> ports enabled at the same time has been achieved by a hack in the ICU > >> driver(1) that faked the use of the two interrupts, no matter which > >> SATA port was in use. > >> > >> Now that the SATA core is ready to handle more than one interrupt, > >> this series adds support for it in the libahci_platform code. The > >> CP110 device tree must be updated to reflect the two SATA ports > >> available and their respective interrupts. To do not break DT backward > >> compatibility, the ahci_platform driver now embeds a special quirk > >> which checks if the DT is valid (only for A8k compatible) and, if > >> needed, creates the two missing sub-nodes, and assign them the > >> relevant "reg" and "interrupts" properties, before removing the main > >> SATA node "interrupts" one. > >> > >> (1) The ICU is an irqchip aggregating the CP110 (south-bridge) > >> interrupts into MSIs for the AP806 (north-bridge). > >> > >> Best > >> Sven > >> > >> Change from v2: > >> * Fix commit message of custom irq init for host init > >> > >> Change from v1: > >> * Add a patch to enable custom irq initialization in > >> plattform init host > >> * Add multi_irq_host_ack callback for the msi irq handler > >> * Rework the ahci mvebu patch to initiate the irq and use > >> the new multi_irq_host_ack to handle the custom irq code. > >> Remove the custom irq handler and duplicate code. > >> * Fix the armada8k backwards compatibility code > >> * Rename AHCI_PLATFORM_A8K_QUIRK to AHCI_PLATFORM_ARMADA8K_QUIRK > >> > >> Miquel Raynal (5): > >> ata: ahci: mvebu: Rename a platform data flag > >> ata: ahci: mvebu: Support A8k compatible > >> irqchip/irq-mvebu-icu: Remove the double SATA ports interrupt hack > >> dt-bindings: ata: Update ahci bindings with possible per-port > >> interrupts > >> dt-bindings: ata: Update ahci_mvebu bindings > >> > >> Sven Auhagen (4): > >> ata: libahci_platform: Do not try to get an IRQ when > >> AHCI_HFLAG_MULTI_MSI is set > >> ata: ahci: add ack callback to multi irq handler > >> ata: ahci: mvebu: Add support for A8k legacy DT bindings > >> arm64: dts: marvell: armada-cp110: Switch to per-port SATA interrupts > >> > >> .../devicetree/bindings/ata/ahci-platform.txt | 7 + > >> arch/arm64/boot/dts/marvell/armada-cp11x.dtsi | 6 +- > >> drivers/ata/ahci.h | 2 + > >> drivers/ata/ahci_mvebu.c | 143 ++++++++++++++++-- > >> drivers/ata/libahci.c | 4 + > >> drivers/ata/libahci_platform.c | 19 ++- > >> drivers/irqchip/irq-mvebu-icu.c | 18 --- > >> include/linux/ahci_platform.h | 1 + > >> 8 files changed, 160 insertions(+), 40 deletions(-) > >> > >> -- > >> 2.20.1 > >> > >> > > -- > Gregory Clement, Bootlin > Embedded Linux and Kernel engineering > https://eur03.safelinks.protection.outlook.com/?url=http%3A%2F%2Fbootlin.com%2F&data=04%7C01%7Csven.auhagen%40voleatech.de%7C6500487ba77641ea9ae508d8eaa842b1%7Cb82a99f679814a7295344d35298f847b%7C0%7C0%7C637517355700947932%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000&sdata=eP%2FQEvJ2oUeHeA0DH1N2yxp0CjRZLV50LJ9QbF9nDDs%3D&reserved=0
On Fri, 19 Mar 2021 08:08:34 +0000, Marcin Wojtas <mw@semihalf.com> wrote: > > HI Gregory, > > pt., 19 mar 2021 o 08:35 Gregory CLEMENT <gregory.clement@bootlin.com> > napisał(a): > > > > Hello Marcin, > > > > > [Resend in plain text] > > > > > > Hi, > > > > > > Just letting everyone know - merging only the DT part of this patchset > > > broke AHCI on all Marvell Armada 7k8k / CN913x platforms in v5.11 > > > release. > > > > It's unfortunate that we didn't know this when v5.11-rc1 was > > released. However it is still time for a fix, I will submit it. > > As I explained in the other email when I applied this I really though > > that the driver part will be applied, I don't know what happened here. > > > > Sure, looking at the thread it looks more of a communication issue. I > am also surprised the breakage went unnoticed for a while (unless > everyone is using edk2, like myself :) ). I think it would be good to > revert the change on top of v5.11.x. The drivers adoption would have to > land before v5.12 though, so that not to repeat the problem during next release. > > Small rant: > A general issue with the DT binding changes of this kind (previously > clocks, ICU, etc.) that I have, is a side effect of incompatibility > with older kernels/other OSs. The latter must follow the > modifications, but you can forget of booting e.g. Debian Buster with > the ToT device tree. Therefore in edk2 I do not update the device tree > fork to often and need to tweak it in order to have the widest support > coverage. Unfortunately, this has been the case for this machine since it became available. I can happily boot any kernel on other systems of the same vintage without touching anything firmware related, which is crucial to identify regressions. The A8k requires instead a per-kernel DT, something that only works if you treat it as an embedded system, and not a standard system (which is why mine has been collecting dust for some time now). I don't think the maintainers have ever been interested in solving this problem. As for ACPI, that'd probably be the best thing that can happen to this platform. Not sure that's remotely possible though, given how "interesting" the HW is. M.
Hi Marc, pt., 19 mar 2021 o 10:33 Marc Zyngier <maz@kernel.org> napisał(a): > > On Fri, 19 Mar 2021 08:08:34 +0000, > Marcin Wojtas <mw@semihalf.com> wrote: > > > > HI Gregory, > > > > pt., 19 mar 2021 o 08:35 Gregory CLEMENT <gregory.clement@bootlin.com> > > napisał(a): > > > > > > Hello Marcin, > > > > > > > [Resend in plain text] > > > > > > > > Hi, > > > > > > > > Just letting everyone know - merging only the DT part of this patchset > > > > broke AHCI on all Marvell Armada 7k8k / CN913x platforms in v5.11 > > > > release. > > > > > > It's unfortunate that we didn't know this when v5.11-rc1 was > > > released. However it is still time for a fix, I will submit it. > > > As I explained in the other email when I applied this I really though > > > that the driver part will be applied, I don't know what happened here. > > > > > > > Sure, looking at the thread it looks more of a communication issue. I > > am also surprised the breakage went unnoticed for a while (unless > > everyone is using edk2, like myself :) ). I think it would be good to > > revert the change on top of v5.11.x. The drivers adoption would have to > > land before v5.12 though, so that not to repeat the problem during next release. > > > > Small rant: > > A general issue with the DT binding changes of this kind (previously > > clocks, ICU, etc.) that I have, is a side effect of incompatibility > > with older kernels/other OSs. The latter must follow the > > modifications, but you can forget of booting e.g. Debian Buster with > > the ToT device tree. Therefore in edk2 I do not update the device tree > > fork to often and need to tweak it in order to have the widest support > > coverage. > > Unfortunately, this has been the case for this machine since it became > available. I can happily boot any kernel on other systems of the same > vintage without touching anything firmware related, which is crucial > to identify regressions. > > The A8k requires instead a per-kernel DT, something that only works if > you treat it as an embedded system, and not a standard system (which > is why mine has been collecting dust for some time now). I don't think > the maintainers have ever been interested in solving this problem. > > As for ACPI, that'd probably be the best thing that can happen to this > platform. Not sure that's remotely possible though, given how > "interesting" the HW is. ACPI has been up and running for this platform for a couple of years now, on MacchiatoBin you get USB, SATA, network ports, PCIE and even SD/MMC (the latter was merged in v5.11 kernel). Best regards, Marcin > > -- > Without deviation from the norm, progress is not possible.
> From: Marcin Wojtas <mw@semihalf.com> > Date: Fri, 19 Mar 2021 10:48:08 +0100 > > Hi Marc, > > pt., 19 mar 2021 o 10:33 Marc Zyngier <maz@kernel.org> napisał(a): > > > > On Fri, 19 Mar 2021 08:08:34 +0000, > > Marcin Wojtas <mw@semihalf.com> wrote: > > > > > > HI Gregory, > > > > > > pt., 19 mar 2021 o 08:35 Gregory CLEMENT <gregory.clement@bootlin.com> > > > napisał(a): > > > > > > > > Hello Marcin, > > > > > > > > > [Resend in plain text] > > > > > > > > > > Hi, > > > > > > > > > > Just letting everyone know - merging only the DT part of this patchset > > > > > broke AHCI on all Marvell Armada 7k8k / CN913x platforms in v5.11 > > > > > release. > > > > > > > > It's unfortunate that we didn't know this when v5.11-rc1 was > > > > released. However it is still time for a fix, I will submit it. > > > > As I explained in the other email when I applied this I really though > > > > that the driver part will be applied, I don't know what happened here. > > > > > > > > > > Sure, looking at the thread it looks more of a communication issue. I > > > am also surprised the breakage went unnoticed for a while (unless > > > everyone is using edk2, like myself :) ). I think it would be good to > > > revert the change on top of v5.11.x. The drivers adoption would have to > > > land before v5.12 though, so that not to repeat the problem during next release. > > > > > > Small rant: > > > A general issue with the DT binding changes of this kind (previously > > > clocks, ICU, etc.) that I have, is a side effect of incompatibility > > > with older kernels/other OSs. The latter must follow the > > > modifications, but you can forget of booting e.g. Debian Buster with > > > the ToT device tree. Therefore in edk2 I do not update the device tree > > > fork to often and need to tweak it in order to have the widest support > > > coverage. > > > > Unfortunately, this has been the case for this machine since it became > > available. I can happily boot any kernel on other systems of the same > > vintage without touching anything firmware related, which is crucial > > to identify regressions. > > > > The A8k requires instead a per-kernel DT, something that only works if > > you treat it as an embedded system, and not a standard system (which > > is why mine has been collecting dust for some time now). I don't think > > the maintainers have ever been interested in solving this problem. > > > > As for ACPI, that'd probably be the best thing that can happen to this > > platform. Not sure that's remotely possible though, given how > > "interesting" the HW is. > > ACPI has been up and running for this platform for a couple of years > now, on MacchiatoBin you get USB, SATA, network ports, PCIE and even > SD/MMC (the latter was merged in v5.11 kernel). Unfortunately ACPI doesn't describe how the SFP cages are connected. As soon as GPIOs and I2C busses are involved device trees are so much better than ACPI, and I wouldn't really be interested in adding the necessary OS support the network ports with ACPI in OpenBSD. Cheers, Mark
> Unfortunately ACPI doesn't describe how the SFP cages are connected. Nor MDIO busses and Ethernet PHYs. ACPI is missing a lot of support for basic elements of networking. I guess this is because it expects firmware in the NIC to be controlling these parts, not Linux. > > As soon as GPIOs and I2C busses are involved device trees are so much > better than ACPI Yes, ACPI has along way to go before it is usable for SoCs with integrated Ethernet. Andrew
Hi Andrew, pt., 19 mar 2021 o 21:53 Andrew Lunn <andrew@lunn.ch> napisał(a): > > > Unfortunately ACPI doesn't describe how the SFP cages are connected. > > Nor MDIO busses and Ethernet PHYs. > > ACPI is missing a lot of support for basic elements of networking. I > guess this is because it expects firmware in the NIC to be controlling > these parts, not Linux. > > > > > As soon as GPIOs and I2C busses are involved device trees are so much > > better than ACPI > > Yes, ACPI has along way to go before it is usable for SoCs with > integrated Ethernet. > I agree, however hopefully the MDIO + ACPI patchset, that currently seems to be pretty advanced on the lists, will land - it will be a big step forward and open a path for full SFP support. However, my point is, that in case of MacchiatoBin, I prefer to use a bit limited link IRQs, just for having a guarantee to boot each Linux kernel with mvpp2 starting from v4.16 until today and being sure to install various distros/OSs/ESXI. For DT one have to find a matching version, that luckily won't break the boot. Of course the DT is more flexible and offers huge possibilities (e.g. DSA), but the way it's been done, inherited straight from armv7 embedded-hacking times is imo a burden. Unstable, constantly changing ABI, extending drivers rather than describe HW is too often a problem. This patchset is a bright example that there is a field for improvement. In my ideal world DT should be properly designed (vendors+community), out of Linux tree, reviewed by maintainers from different projects, backward compatible and ensuring proper information/heads-up when planning to do a meaningful change. I know how it looks and I'm aware it is hard, but we should at least try :) Best regards, Marcin
> I agree, however hopefully the MDIO + ACPI patchset, that currently > seems to be pretty advanced on the lists, will land Well, it has not yet addressed the standing NACK. Which is why no networking people are reviewing it. We will have to wait and see if they can do what it needed to clear the NACK. Andrew
From: Sven Auhagen <sven.auhagen@voleatech.de> Hello, There were already 4 versions of this series from Miquèl. I talked to Miquèl and I fixed up the last comments from v4. I am looking for feedback if this patch series is now ready to be merged and what should be further changed. Here is the original cover letter: Some time ago, when the initial support for Armada CP110 was contributed, the SATA core was not able to handle per-port interrupts. Despite the hardware reality, the device tree only represents one main interrupt for the two ports. Having both SATA ports enabled at the same time has been achieved by a hack in the ICU driver(1) that faked the use of the two interrupts, no matter which SATA port was in use. Now that the SATA core is ready to handle more than one interrupt, this series adds support for it in the libahci_platform code. The CP110 device tree must be updated to reflect the two SATA ports available and their respective interrupts. To do not break DT backward compatibility, the ahci_platform driver now embeds a special quirk which checks if the DT is valid (only for A8k compatible) and, if needed, creates the two missing sub-nodes, and assign them the relevant "reg" and "interrupts" properties, before removing the main SATA node "interrupts" one. (1) The ICU is an irqchip aggregating the CP110 (south-bridge) interrupts into MSIs for the AP806 (north-bridge). Best Sven Change from v2: * Fix commit message of custom irq init for host init Change from v1: * Add a patch to enable custom irq initialization in plattform init host * Add multi_irq_host_ack callback for the msi irq handler * Rework the ahci mvebu patch to initiate the irq and use the new multi_irq_host_ack to handle the custom irq code. Remove the custom irq handler and duplicate code. * Fix the armada8k backwards compatibility code * Rename AHCI_PLATFORM_A8K_QUIRK to AHCI_PLATFORM_ARMADA8K_QUIRK Miquel Raynal (5): ata: ahci: mvebu: Rename a platform data flag ata: ahci: mvebu: Support A8k compatible irqchip/irq-mvebu-icu: Remove the double SATA ports interrupt hack dt-bindings: ata: Update ahci bindings with possible per-port interrupts dt-bindings: ata: Update ahci_mvebu bindings Sven Auhagen (4): ata: libahci_platform: Do not try to get an IRQ when AHCI_HFLAG_MULTI_MSI is set ata: ahci: add ack callback to multi irq handler ata: ahci: mvebu: Add support for A8k legacy DT bindings arm64: dts: marvell: armada-cp110: Switch to per-port SATA interrupts .../devicetree/bindings/ata/ahci-platform.txt | 7 + arch/arm64/boot/dts/marvell/armada-cp11x.dtsi | 6 +- drivers/ata/ahci.h | 2 + drivers/ata/ahci_mvebu.c | 143 ++++++++++++++++-- drivers/ata/libahci.c | 4 + drivers/ata/libahci_platform.c | 19 ++- drivers/irqchip/irq-mvebu-icu.c | 18 --- include/linux/ahci_platform.h | 1 + 8 files changed, 160 insertions(+), 40 deletions(-)