| Message ID | 1314602339-18392-2-git-send-email-richard.zhu@linaro.org |
|---|---|
| State | Not Applicable |
| Delegated to: | David Miller |
| Headers | show |
On Mon, Aug 29, 2011 at 3:18 PM, Richard Zhu <richard.zhu@linaro.org> wrote: > On imx53 AHCI, soft reset fails with IPMS set when PMP > is enabled but SATA HDD/ODD is connected to SATA port, > do soft reset again to port 0. > So the 'ahci_pmp_retry_srst_ops' is required when imx53 > ahci is present. > > Signed-off-by: Richard Zhu <richard.zhu@linaro.org> Looks good to me, I like this way very much. Acked-by: Eric Miao <eric.miao@linaro.org> > --- > drivers/ata/ahci_platform.c | 44 +++++++++++++++++++++++++++++++++++++----- > 1 files changed, 38 insertions(+), 6 deletions(-) > > diff --git a/drivers/ata/ahci_platform.c b/drivers/ata/ahci_platform.c > index 6fef1fa..f32c91e 100644 > --- a/drivers/ata/ahci_platform.c > +++ b/drivers/ata/ahci_platform.c > @@ -23,6 +23,41 @@ > #include <linux/ahci_platform.h> > #include "ahci.h" > > +enum ahci_type { > + AHCI, /* standard platform ahci */ > + IMX53_AHCI, /* ahci on i.mx53 */ > +}; > + > +static struct platform_device_id ahci_devtype[] = { > + { > + .name = "ahci", > + .driver_data = AHCI, > + }, { > + .name = "imx53-ahci", > + .driver_data = IMX53_AHCI, > + }, { > + /* sentinel */ > + } > +}; > +MODULE_DEVICE_TABLE(platform, ahci_devtype); > + > + > +static const struct ata_port_info ahci_port_info[] = { > + /* by features */ > + [AHCI] = { > + .flags = AHCI_FLAG_COMMON, > + .pio_mask = ATA_PIO4, > + .udma_mask = ATA_UDMA6, > + .port_ops = &ahci_ops, > + }, > + [IMX53_AHCI] = { > + .flags = AHCI_FLAG_COMMON, > + .pio_mask = ATA_PIO4, > + .udma_mask = ATA_UDMA6, > + .port_ops = &ahci_pmp_retry_srst_ops, > + }, > +}; > + > static struct scsi_host_template ahci_platform_sht = { > AHCI_SHT("ahci_platform"), > }; > @@ -31,12 +66,8 @@ static int __init ahci_probe(struct platform_device *pdev) > { > struct device *dev = &pdev->dev; > struct ahci_platform_data *pdata = dev->platform_data; > - struct ata_port_info pi = { > - .flags = AHCI_FLAG_COMMON, > - .pio_mask = ATA_PIO4, > - .udma_mask = ATA_UDMA6, > - .port_ops = &ahci_ops, > - }; > + struct platform_device_id *id_entry = platform_get_device_id(pdev); > + struct ata_port_info pi = ahci_port_info[id_entry->driver_data]; > const struct ata_port_info *ppi[] = { &pi, NULL }; > struct ahci_host_priv *hpriv; > struct ata_host *host; > @@ -177,6 +208,7 @@ static struct platform_driver ahci_driver = { > .name = "ahci", > .owner = THIS_MODULE, > }, > + .id_table = ahci_devtype, > }; > > static int __init ahci_init(void) > -- > 1.7.1 > > > > _______________________________________________ > linux-arm-kernel mailing list > linux-arm-kernel@lists.infradead.org > http://lists.infradead.org/mailman/listinfo/linux-arm-kernel > -- To unsubscribe from this list: send the line "unsubscribe linux-ide" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Mon, Aug 29, 2011 at 03:18:55PM +0800, Richard Zhu wrote: > On imx53 AHCI, soft reset fails with IPMS set when PMP > is enabled but SATA HDD/ODD is connected to SATA port, > do soft reset again to port 0. > So the 'ahci_pmp_retry_srst_ops' is required when imx53 > ahci is present. > > Signed-off-by: Richard Zhu <richard.zhu@linaro.org> > --- > drivers/ata/ahci_platform.c | 44 +++++++++++++++++++++++++++++++++++++----- > 1 files changed, 38 insertions(+), 6 deletions(-) > > diff --git a/drivers/ata/ahci_platform.c b/drivers/ata/ahci_platform.c > index 6fef1fa..f32c91e 100644 > --- a/drivers/ata/ahci_platform.c > +++ b/drivers/ata/ahci_platform.c > @@ -23,6 +23,41 @@ > #include <linux/ahci_platform.h> > #include "ahci.h" > > +enum ahci_type { > + AHCI, /* standard platform ahci */ > + IMX53_AHCI, /* ahci on i.mx53 */ How about making all the IMX*-naming more generic because other SoC might need this somewhen, too?
On Mon, Aug 29, 2011 at 4:36 PM, Wolfram Sang <w.sang@pengutronix.de> wrote: > On Mon, Aug 29, 2011 at 03:18:55PM +0800, Richard Zhu wrote: >> On imx53 AHCI, soft reset fails with IPMS set when PMP >> is enabled but SATA HDD/ODD is connected to SATA port, >> do soft reset again to port 0. >> So the 'ahci_pmp_retry_srst_ops' is required when imx53 >> ahci is present. >> >> Signed-off-by: Richard Zhu <richard.zhu@linaro.org> >> --- >> drivers/ata/ahci_platform.c | 44 +++++++++++++++++++++++++++++++++++++----- >> 1 files changed, 38 insertions(+), 6 deletions(-) >> >> diff --git a/drivers/ata/ahci_platform.c b/drivers/ata/ahci_platform.c >> index 6fef1fa..f32c91e 100644 >> --- a/drivers/ata/ahci_platform.c >> +++ b/drivers/ata/ahci_platform.c >> @@ -23,6 +23,41 @@ >> #include <linux/ahci_platform.h> >> #include "ahci.h" >> >> +enum ahci_type { >> + AHCI, /* standard platform ahci */ >> + IMX53_AHCI, /* ahci on i.mx53 */ > > How about making all the IMX*-naming more generic because other SoC might need > this somewhen, too? From a practical point of view, we can start with what we know already. As the SATA controller on "other SoC" so far, we don't know yet if they show the same issue (which we have to use ahci_pmp_retry_srst_ops). So Wolfram, how about we consider a generic one once more controllers are being added? > > -- > Pengutronix e.K. | Wolfram Sang | > Industrial Linux Solutions | http://www.pengutronix.de/ | > > _______________________________________________ > linux-arm-kernel mailing list > linux-arm-kernel@lists.infradead.org > http://lists.infradead.org/mailman/listinfo/linux-arm-kernel > > -- To unsubscribe from this list: send the line "unsubscribe linux-ide" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Mon, Aug 29, 2011 at 04:51:46PM +0800, Eric Miao wrote: > On Mon, Aug 29, 2011 at 4:36 PM, Wolfram Sang <w.sang@pengutronix.de> wrote: > > On Mon, Aug 29, 2011 at 03:18:55PM +0800, Richard Zhu wrote: > >> On imx53 AHCI, soft reset fails with IPMS set when PMP > >> is enabled but SATA HDD/ODD is connected to SATA port, > >> do soft reset again to port 0. > >> So the 'ahci_pmp_retry_srst_ops' is required when imx53 > >> ahci is present. > >> > >> Signed-off-by: Richard Zhu <richard.zhu@linaro.org> > >> --- > >> drivers/ata/ahci_platform.c | 44 +++++++++++++++++++++++++++++++++++++----- > >> 1 files changed, 38 insertions(+), 6 deletions(-) > >> > >> diff --git a/drivers/ata/ahci_platform.c b/drivers/ata/ahci_platform.c > >> index 6fef1fa..f32c91e 100644 > >> --- a/drivers/ata/ahci_platform.c > >> +++ b/drivers/ata/ahci_platform.c > >> @@ -23,6 +23,41 @@ > >> #include <linux/ahci_platform.h> > >> #include "ahci.h" > >> > >> +enum ahci_type { > >> + AHCI, /* standard platform ahci */ > >> + IMX53_AHCI, /* ahci on i.mx53 */ > > > > How about making all the IMX*-naming more generic because other SoC might need > > this somewhen, too? > > From a practical point of view, we can start with what we know already. > As the SATA controller on "other SoC" so far, we don't know yet if they > show the same issue (which we have to use ahci_pmp_retry_srst_ops). > > So Wolfram, how about we consider a generic one once more controllers > are being added? If we do so later, and we then also change the platform_device_id to something generic, we then also have to change all users, too. Is it so bad to change IMX53_AHCI (and imx53-ahci and alike) to something like "ahci-pmp-retry-srst" (or similar) now? Regards, Wolfram
On Mon, Aug 29, 2011 at 5:37 PM, Wolfram Sang <w.sang@pengutronix.de> wrote: > On Mon, Aug 29, 2011 at 04:51:46PM +0800, Eric Miao wrote: >> On Mon, Aug 29, 2011 at 4:36 PM, Wolfram Sang <w.sang@pengutronix.de> wrote: >> > On Mon, Aug 29, 2011 at 03:18:55PM +0800, Richard Zhu wrote: >> >> On imx53 AHCI, soft reset fails with IPMS set when PMP >> >> is enabled but SATA HDD/ODD is connected to SATA port, >> >> do soft reset again to port 0. >> >> So the 'ahci_pmp_retry_srst_ops' is required when imx53 >> >> ahci is present. >> >> >> >> Signed-off-by: Richard Zhu <richard.zhu@linaro.org> >> >> --- >> >> drivers/ata/ahci_platform.c | 44 +++++++++++++++++++++++++++++++++++++----- >> >> 1 files changed, 38 insertions(+), 6 deletions(-) >> >> >> >> diff --git a/drivers/ata/ahci_platform.c b/drivers/ata/ahci_platform.c >> >> index 6fef1fa..f32c91e 100644 >> >> --- a/drivers/ata/ahci_platform.c >> >> +++ b/drivers/ata/ahci_platform.c >> >> @@ -23,6 +23,41 @@ >> >> #include <linux/ahci_platform.h> >> >> #include "ahci.h" >> >> >> >> +enum ahci_type { >> >> + AHCI, /* standard platform ahci */ >> >> + IMX53_AHCI, /* ahci on i.mx53 */ >> > >> > How about making all the IMX*-naming more generic because other SoC might need >> > this somewhen, too? >> >> From a practical point of view, we can start with what we know already. >> As the SATA controller on "other SoC" so far, we don't know yet if they >> show the same issue (which we have to use ahci_pmp_retry_srst_ops). >> >> So Wolfram, how about we consider a generic one once more controllers >> are being added? > > If we do so later, and we then also change the platform_device_id to something > generic, we then also have to change all users, too. Is it so bad to change > IMX53_AHCI (and imx53-ahci and alike) to something like "ahci-pmp-retry-srst" > (or similar) now? There are other fields in ata_port_info which might be different between boards. Using a SoC specific name as an index to an array of ata_port_info will be more flexible, as is adopted in drivers/ata/ahci.c - the PCI driver. -- To unsubscribe from this list: send the line "unsubscribe linux-ide" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Mon, Aug 29, 2011 at 05:54:23PM +0800, Eric Miao wrote: > On Mon, Aug 29, 2011 at 5:37 PM, Wolfram Sang <w.sang@pengutronix.de> wrote: > > On Mon, Aug 29, 2011 at 04:51:46PM +0800, Eric Miao wrote: > >> On Mon, Aug 29, 2011 at 4:36 PM, Wolfram Sang <w.sang@pengutronix.de> wrote: > >> > On Mon, Aug 29, 2011 at 03:18:55PM +0800, Richard Zhu wrote: > >> >> On imx53 AHCI, soft reset fails with IPMS set when PMP > >> >> is enabled but SATA HDD/ODD is connected to SATA port, > >> >> do soft reset again to port 0. > >> >> So the 'ahci_pmp_retry_srst_ops' is required when imx53 > >> >> ahci is present. > >> >> > >> >> Signed-off-by: Richard Zhu <richard.zhu@linaro.org> > >> >> --- > >> >> drivers/ata/ahci_platform.c | 44 +++++++++++++++++++++++++++++++++++++----- > >> >> 1 files changed, 38 insertions(+), 6 deletions(-) > >> >> > >> >> diff --git a/drivers/ata/ahci_platform.c b/drivers/ata/ahci_platform.c > >> >> index 6fef1fa..f32c91e 100644 > >> >> --- a/drivers/ata/ahci_platform.c > >> >> +++ b/drivers/ata/ahci_platform.c > >> >> @@ -23,6 +23,41 @@ > >> >> #include <linux/ahci_platform.h> > >> >> #include "ahci.h" > >> >> > >> >> +enum ahci_type { > >> >> + AHCI, /* standard platform ahci */ > >> >> + IMX53_AHCI, /* ahci on i.mx53 */ > >> > > >> > How about making all the IMX*-naming more generic because other SoC might need > >> > this somewhen, too? > >> > >> From a practical point of view, we can start with what we know already. > >> As the SATA controller on "other SoC" so far, we don't know yet if they > >> show the same issue (which we have to use ahci_pmp_retry_srst_ops). > >> > >> So Wolfram, how about we consider a generic one once more controllers > >> are being added? > > > > If we do so later, and we then also change the platform_device_id to something > > generic, we then also have to change all users, too. Is it so bad to change > > IMX53_AHCI (and imx53-ahci and alike) to something like "ahci-pmp-retry-srst" > > (or similar) now? > > There are other fields in ata_port_info which might be different between > boards. Using a SoC specific name as an index to an array of ata_port_info > will be more flexible, as is adopted in drivers/ata/ahci.c - the PCI driver. Well, that file also has generic fixups: [board_ahci_ign_iferr] = ... which is what I had in mind thinking that the flaw was not too imx53 specific. If you think I am wrong, then be it. I mainly wanted to avoid the fixups which might be needed later. Regards, Wolfram
Hello, On Mon, Aug 29, 2011 at 03:18:55PM +0800, Richard Zhu wrote: > On imx53 AHCI, soft reset fails with IPMS set when PMP > is enabled but SATA HDD/ODD is connected to SATA port, > do soft reset again to port 0. > So the 'ahci_pmp_retry_srst_ops' is required when imx53 > ahci is present. > > Signed-off-by: Richard Zhu <richard.zhu@linaro.org> > --- [...] > struct device *dev = &pdev->dev; > struct ahci_platform_data *pdata = dev->platform_data; > - struct ata_port_info pi = { > - .flags = AHCI_FLAG_COMMON, > - .pio_mask = ATA_PIO4, > - .udma_mask = ATA_UDMA6, > - .port_ops = &ahci_ops, > - }; > + struct platform_device_id *id_entry = platform_get_device_id(pdev); > + struct ata_port_info pi = ahci_port_info[id_entry->driver_data]; Why not pass port info via platform_data? It seems to be platform specific nowadays, so leave the default as is, but let the platforms pass their own port info through platform_data. Thanks,
On Mon, Aug 29, 2011 at 8:12 PM, Anton Vorontsov <cbouatmailru@gmail.com> wrote: > Hello, > > On Mon, Aug 29, 2011 at 03:18:55PM +0800, Richard Zhu wrote: >> On imx53 AHCI, soft reset fails with IPMS set when PMP >> is enabled but SATA HDD/ODD is connected to SATA port, >> do soft reset again to port 0. >> So the 'ahci_pmp_retry_srst_ops' is required when imx53 >> ahci is present. >> >> Signed-off-by: Richard Zhu <richard.zhu@linaro.org> >> --- > [...] >> struct device *dev = &pdev->dev; >> struct ahci_platform_data *pdata = dev->platform_data; >> - struct ata_port_info pi = { >> - .flags = AHCI_FLAG_COMMON, >> - .pio_mask = ATA_PIO4, >> - .udma_mask = ATA_UDMA6, >> - .port_ops = &ahci_ops, >> - }; >> + struct platform_device_id *id_entry = platform_get_device_id(pdev); >> + struct ata_port_info pi = ahci_port_info[id_entry->driver_data]; > > Why not pass port info via platform_data? It seems to be platform > specific nowadays, so leave the default as is, but let the platforms > pass their own port info through platform_data. That's also a very clean way. However I have the concern that it might end up with many duplicate entries. > > Thanks, > > -- > Anton Vorontsov > Email: cbouatmailru@gmail.com > > _______________________________________________ > linux-arm-kernel mailing list > linux-arm-kernel@lists.infradead.org > http://lists.infradead.org/mailman/listinfo/linux-arm-kernel > -- To unsubscribe from this list: send the line "unsubscribe linux-ide" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Hi: Pass port info from the platform data is a good idea. About the codes duplication in the board level, I think that we can re-use the default port info on most boards, when the ata_port_info is null in the platform data struct. We can leave the ahci_platform.c without any modifications in this way. How do you guys think about? Best Regard Richard Zhu On 29 August 2011 20:25, Eric Miao <eric.miao@linaro.org> wrote: > On Mon, Aug 29, 2011 at 8:12 PM, Anton Vorontsov <cbouatmailru@gmail.com> wrote: >> Hello, >> >> On Mon, Aug 29, 2011 at 03:18:55PM +0800, Richard Zhu wrote: >>> On imx53 AHCI, soft reset fails with IPMS set when PMP >>> is enabled but SATA HDD/ODD is connected to SATA port, >>> do soft reset again to port 0. >>> So the 'ahci_pmp_retry_srst_ops' is required when imx53 >>> ahci is present. >>> >>> Signed-off-by: Richard Zhu <richard.zhu@linaro.org> >>> --- >> [...] >>> struct device *dev = &pdev->dev; >>> struct ahci_platform_data *pdata = dev->platform_data; >>> - struct ata_port_info pi = { >>> - .flags = AHCI_FLAG_COMMON, >>> - .pio_mask = ATA_PIO4, >>> - .udma_mask = ATA_UDMA6, >>> - .port_ops = &ahci_ops, >>> - }; >>> + struct platform_device_id *id_entry = platform_get_device_id(pdev); >>> + struct ata_port_info pi = ahci_port_info[id_entry->driver_data]; >> >> Why not pass port info via platform_data? It seems to be platform >> specific nowadays, so leave the default as is, but let the platforms >> pass their own port info through platform_data. > > That's also a very clean way. However I have the concern that it might > end up with many duplicate entries. > >> >> Thanks, >> >> -- >> Anton Vorontsov >> Email: cbouatmailru@gmail.com >> >> _______________________________________________ >> linux-arm-kernel mailing list >> linux-arm-kernel@lists.infradead.org >> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel >> > -- To unsubscribe from this list: send the line "unsubscribe linux-ide" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Tue, Aug 30, 2011 at 2:21 PM, Richard Zhu <richard.zhu@linaro.org> wrote: > Hi: > Pass port info from the platform data is a good idea. > About the codes duplication in the board level, I think that we can > re-use the default port info on most boards, when the ata_port_info > is null in the platform data struct. > We can leave the ahci_platform.c without any modifications in this way. > > How do you guys think about? +1. Richard, please let's first go that way, so to have minimum impact to ahci_platform.c -- To unsubscribe from this list: send the line "unsubscribe linux-ide" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Hi: One more concern, the .../drivers/ata/ahci.h file should be moved to .../include/linux/ folder. Because that the kinds of macros used in the ata_port_info and the ahci_pmp_retry_srst_ops are all defined in the .../drivers/ata/ahci.h file. Otherwise, the ata_port_info can't be defined in the board related platform data. How we can handle this case? Best Regard Richard Zhu On 30 August 2011 14:21, Richard Zhu <richard.zhu@linaro.org> wrote: > Hi: > Pass port info from the platform data is a good idea. > About the codes duplication in the board level, I think that we can > re-use the default port info on most boards, when the ata_port_info > is null in the platform data struct. > We can leave the ahci_platform.c without any modifications in this way. > > How do you guys think about? > > Best Regard > Richard Zhu > > > On 29 August 2011 20:25, Eric Miao <eric.miao@linaro.org> wrote: >> On Mon, Aug 29, 2011 at 8:12 PM, Anton Vorontsov <cbouatmailru@gmail.com> wrote: >>> Hello, >>> >>> On Mon, Aug 29, 2011 at 03:18:55PM +0800, Richard Zhu wrote: >>>> On imx53 AHCI, soft reset fails with IPMS set when PMP >>>> is enabled but SATA HDD/ODD is connected to SATA port, >>>> do soft reset again to port 0. >>>> So the 'ahci_pmp_retry_srst_ops' is required when imx53 >>>> ahci is present. >>>> >>>> Signed-off-by: Richard Zhu <richard.zhu@linaro.org> >>>> --- >>> [...] >>>> struct device *dev = &pdev->dev; >>>> struct ahci_platform_data *pdata = dev->platform_data; >>>> - struct ata_port_info pi = { >>>> - .flags = AHCI_FLAG_COMMON, >>>> - .pio_mask = ATA_PIO4, >>>> - .udma_mask = ATA_UDMA6, >>>> - .port_ops = &ahci_ops, >>>> - }; >>>> + struct platform_device_id *id_entry = platform_get_device_id(pdev); >>>> + struct ata_port_info pi = ahci_port_info[id_entry->driver_data]; >>> >>> Why not pass port info via platform_data? It seems to be platform >>> specific nowadays, so leave the default as is, but let the platforms >>> pass their own port info through platform_data. >> >> That's also a very clean way. However I have the concern that it might >> end up with many duplicate entries. >> >>> >>> Thanks, >>> >>> -- >>> Anton Vorontsov >>> Email: cbouatmailru@gmail.com >>> >>> _______________________________________________ >>> linux-arm-kernel mailing list >>> linux-arm-kernel@lists.infradead.org >>> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel >>> >> > -- To unsubscribe from this list: send the line "unsubscribe linux-ide" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Hi: Maybe we have to come back to take wolfram's suggestion. Remove the mx53 specification in the ahci_type, and replace it by the generic one AHCI_PMP_RTY_SRST Then the other ahci controller that needs the same PMP retry soft reset SW work-around can reuse the ata_port_info. Hi Eric: How about to use the generic ahci_type AHCI_PMP_RTY_SRST to replace the IMX53_AHCI firstly? The specified ahci_type can be added if the ahci controller needs some different configurations refer to the definitions in the AHCI_PMP_RTY_SRST's ata_port_info. In this way, we can re-use the codes as much as possible. Best Regards Richard Zhu On 30 August 2011 15:07, Richard Zhu <richard.zhu@linaro.org> wrote: > Hi: > One more concern, the .../drivers/ata/ahci.h file should be moved to > .../include/linux/ folder. > Because that the kinds of macros used in the ata_port_info and the > ahci_pmp_retry_srst_ops are > all defined in the .../drivers/ata/ahci.h file. > Otherwise, the ata_port_info can't be defined in the board related > platform data. > > How we can handle this case? > > Best Regard > Richard Zhu > > On 30 August 2011 14:21, Richard Zhu <richard.zhu@linaro.org> wrote: >> Hi: >> Pass port info from the platform data is a good idea. >> About the codes duplication in the board level, I think that we can >> re-use the default port info on most boards, when the ata_port_info >> is null in the platform data struct. >> We can leave the ahci_platform.c without any modifications in this way. >> >> How do you guys think about? >> >> Best Regard >> Richard Zhu >> >> >> On 29 August 2011 20:25, Eric Miao <eric.miao@linaro.org> wrote: >>> On Mon, Aug 29, 2011 at 8:12 PM, Anton Vorontsov <cbouatmailru@gmail.com> wrote: >>>> Hello, >>>> >>>> On Mon, Aug 29, 2011 at 03:18:55PM +0800, Richard Zhu wrote: >>>>> On imx53 AHCI, soft reset fails with IPMS set when PMP >>>>> is enabled but SATA HDD/ODD is connected to SATA port, >>>>> do soft reset again to port 0. >>>>> So the 'ahci_pmp_retry_srst_ops' is required when imx53 >>>>> ahci is present. >>>>> >>>>> Signed-off-by: Richard Zhu <richard.zhu@linaro.org> >>>>> --- >>>> [...] >>>>> struct device *dev = &pdev->dev; >>>>> struct ahci_platform_data *pdata = dev->platform_data; >>>>> - struct ata_port_info pi = { >>>>> - .flags = AHCI_FLAG_COMMON, >>>>> - .pio_mask = ATA_PIO4, >>>>> - .udma_mask = ATA_UDMA6, >>>>> - .port_ops = &ahci_ops, >>>>> - }; >>>>> + struct platform_device_id *id_entry = platform_get_device_id(pdev); >>>>> + struct ata_port_info pi = ahci_port_info[id_entry->driver_data]; >>>> >>>> Why not pass port info via platform_data? It seems to be platform >>>> specific nowadays, so leave the default as is, but let the platforms >>>> pass their own port info through platform_data. >>> >>> That's also a very clean way. However I have the concern that it might >>> end up with many duplicate entries. >>> >>>> >>>> Thanks, >>>> >>>> -- >>>> Anton Vorontsov >>>> Email: cbouatmailru@gmail.com >>>> >>>> _______________________________________________ >>>> linux-arm-kernel mailing list >>>> linux-arm-kernel@lists.infradead.org >>>> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel >>>> >>> >> > -- To unsubscribe from this list: send the line "unsubscribe linux-ide" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Tue, Aug 30, 2011 at 3:07 PM, Richard Zhu <richard.zhu@linaro.org> wrote: > Hi: > One more concern, the .../drivers/ata/ahci.h file should be moved to > .../include/linux/ folder. > Because that the kinds of macros used in the ata_port_info and the > ahci_pmp_retry_srst_ops are > all defined in the .../drivers/ata/ahci.h file. > Otherwise, the ata_port_info can't be defined in the board related > platform data. > > How we can handle this case? That's indeed an issue, Anton, what's your suggestion? -- To unsubscribe from this list: send the line "unsubscribe linux-ide" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/drivers/ata/ahci_platform.c b/drivers/ata/ahci_platform.c index 6fef1fa..f32c91e 100644 --- a/drivers/ata/ahci_platform.c +++ b/drivers/ata/ahci_platform.c @@ -23,6 +23,41 @@ #include <linux/ahci_platform.h> #include "ahci.h" +enum ahci_type { + AHCI, /* standard platform ahci */ + IMX53_AHCI, /* ahci on i.mx53 */ +}; + +static struct platform_device_id ahci_devtype[] = { + { + .name = "ahci", + .driver_data = AHCI, + }, { + .name = "imx53-ahci", + .driver_data = IMX53_AHCI, + }, { + /* sentinel */ + } +}; +MODULE_DEVICE_TABLE(platform, ahci_devtype); + + +static const struct ata_port_info ahci_port_info[] = { + /* by features */ + [AHCI] = { + .flags = AHCI_FLAG_COMMON, + .pio_mask = ATA_PIO4, + .udma_mask = ATA_UDMA6, + .port_ops = &ahci_ops, + }, + [IMX53_AHCI] = { + .flags = AHCI_FLAG_COMMON, + .pio_mask = ATA_PIO4, + .udma_mask = ATA_UDMA6, + .port_ops = &ahci_pmp_retry_srst_ops, + }, +}; + static struct scsi_host_template ahci_platform_sht = { AHCI_SHT("ahci_platform"), }; @@ -31,12 +66,8 @@ static int __init ahci_probe(struct platform_device *pdev) { struct device *dev = &pdev->dev; struct ahci_platform_data *pdata = dev->platform_data; - struct ata_port_info pi = { - .flags = AHCI_FLAG_COMMON, - .pio_mask = ATA_PIO4, - .udma_mask = ATA_UDMA6, - .port_ops = &ahci_ops, - }; + struct platform_device_id *id_entry = platform_get_device_id(pdev); + struct ata_port_info pi = ahci_port_info[id_entry->driver_data]; const struct ata_port_info *ppi[] = { &pi, NULL }; struct ahci_host_priv *hpriv; struct ata_host *host; @@ -177,6 +208,7 @@ static struct platform_driver ahci_driver = { .name = "ahci", .owner = THIS_MODULE, }, + .id_table = ahci_devtype, }; static int __init ahci_init(void)
On imx53 AHCI, soft reset fails with IPMS set when PMP is enabled but SATA HDD/ODD is connected to SATA port, do soft reset again to port 0. So the 'ahci_pmp_retry_srst_ops' is required when imx53 ahci is present. Signed-off-by: Richard Zhu <richard.zhu@linaro.org> --- drivers/ata/ahci_platform.c | 44 +++++++++++++++++++++++++++++++++++++----- 1 files changed, 38 insertions(+), 6 deletions(-)