| Message ID | 1433428437-6067-1-git-send-email-Vincent.Wan@amd.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Thu, Jun 04, 2015 at 10:33:57PM +0800, Wan ZongShun wrote: > From: Vincent Wan <vincent.wan@amd.com> > > Change this quirk to apply to AMD Carrizo platform. > > Signed-off-by: Vincent Wan <vincent.wan@amd.com> > Signed-off-by: Wan ZongShun <mcuos.com@gmail.com> > Signed-off-by: Wan ZongShun <linux@mcuos.com> > > Tested-by: Nath, Arindam <Arindam.Nath@amd.com> > Tested-by: Ramesh, Ramya <Ramya.Ramesh@amd.com> > --- > drivers/mmc/host/sdhci-pci.c | 25 ++++++++++++++++++++++++- > include/linux/pci_ids.h | 1 + > 2 files changed, 25 insertions(+), 1 deletion(-) > > diff --git a/drivers/mmc/host/sdhci-pci.c b/drivers/mmc/host/sdhci-pci.c > index f208f20..94f54d2 100644 > --- a/drivers/mmc/host/sdhci-pci.c > +++ b/drivers/mmc/host/sdhci-pci.c > @@ -724,14 +724,37 @@ static const struct sdhci_pci_fixes sdhci_rtsx = { > .probe_slot = rtsx_probe_slot, > }; > > +/*AMD chipset generation*/ > +enum amd_chipset_gen { > + AMD_CHIPSET_BEFORE_ML, > + AMD_CHIPSET_CZ, > + AMD_CHIPSET_NL, > + AMD_CHIPSET_UNKNOWN, > +}; > + > static int amd_probe(struct sdhci_pci_chip *chip) > { > struct pci_dev *smbus_dev; > + enum amd_chipset_gen gen; > > smbus_dev = pci_get_device(PCI_VENDOR_ID_AMD, > PCI_DEVICE_ID_AMD_HUDSON2_SMBUS, NULL); > + if (smbus_dev) { > + gen = AMD_CHIPSET_BEFORE_ML; > + } else { > + smbus_dev = pci_get_device(PCI_VENDOR_ID_AMD, > + PCI_DEVICE_ID_AMD_KERNCZ_SMBUS, NULL); > + if (smbus_dev) { > + if (smbus_dev->revision < 0x51) > + gen = AMD_CHIPSET_CZ; > + else > + gen = AMD_CHIPSET_NL; > + } else { > + gen = AMD_CHIPSET_UNKNOWN; > + } > + } > > - if (smbus_dev && (smbus_dev->revision < 0x51)) { > + if ((gen == AMD_CHIPSET_BEFORE_ML) || (gen == AMD_CHIPSET_CZ)) { > chip->quirks2 |= SDHCI_QUIRK2_CLEAR_TRANSFERMODE_REG_BEFORE_CMD; > chip->quirks2 |= SDHCI_QUIRK2_BROKEN_HS200; > } > diff --git a/include/linux/pci_ids.h b/include/linux/pci_ids.h > index 2f7b9a4..cb63a7b 100644 > --- a/include/linux/pci_ids.h > +++ b/include/linux/pci_ids.h > @@ -579,6 +579,7 @@ > #define PCI_DEVICE_ID_AMD_HUDSON2_SATA_IDE 0x7800 > #define PCI_DEVICE_ID_AMD_HUDSON2_SMBUS 0x780b > #define PCI_DEVICE_ID_AMD_HUDSON2_IDE 0x780c > +#define PCI_DEVICE_ID_AMD_KERNCZ_SMBUS 0x790b KERNCZ? Also, that device id is used only in sdhci-pci.c so it should be defined there. Only when multiple drivers/compilation units need it, then it should go to pci_ids.h.
2015-06-04 23:32 GMT+08:00 Borislav Petkov <bp@alien8.de>: > On Thu, Jun 04, 2015 at 10:33:57PM +0800, Wan ZongShun wrote: >> From: Vincent Wan <vincent.wan@amd.com> >> >> Change this quirk to apply to AMD Carrizo platform. >> >> Signed-off-by: Vincent Wan <vincent.wan@amd.com> >> Signed-off-by: Wan ZongShun <mcuos.com@gmail.com> >> Signed-off-by: Wan ZongShun <linux@mcuos.com> >> >> Tested-by: Nath, Arindam <Arindam.Nath@amd.com> >> Tested-by: Ramesh, Ramya <Ramya.Ramesh@amd.com> >> --- >> drivers/mmc/host/sdhci-pci.c | 25 ++++++++++++++++++++++++- >> include/linux/pci_ids.h | 1 + >> 2 files changed, 25 insertions(+), 1 deletion(-) >> >> diff --git a/drivers/mmc/host/sdhci-pci.c b/drivers/mmc/host/sdhci-pci.c >> index f208f20..94f54d2 100644 >> --- a/drivers/mmc/host/sdhci-pci.c >> +++ b/drivers/mmc/host/sdhci-pci.c >> @@ -724,14 +724,37 @@ static const struct sdhci_pci_fixes sdhci_rtsx = { >> .probe_slot = rtsx_probe_slot, >> }; >> >> +/*AMD chipset generation*/ >> +enum amd_chipset_gen { >> + AMD_CHIPSET_BEFORE_ML, >> + AMD_CHIPSET_CZ, >> + AMD_CHIPSET_NL, >> + AMD_CHIPSET_UNKNOWN, >> +}; >> + >> static int amd_probe(struct sdhci_pci_chip *chip) >> { >> struct pci_dev *smbus_dev; >> + enum amd_chipset_gen gen; >> >> smbus_dev = pci_get_device(PCI_VENDOR_ID_AMD, >> PCI_DEVICE_ID_AMD_HUDSON2_SMBUS, NULL); >> + if (smbus_dev) { >> + gen = AMD_CHIPSET_BEFORE_ML; >> + } else { >> + smbus_dev = pci_get_device(PCI_VENDOR_ID_AMD, >> + PCI_DEVICE_ID_AMD_KERNCZ_SMBUS, NULL); >> + if (smbus_dev) { >> + if (smbus_dev->revision < 0x51) >> + gen = AMD_CHIPSET_CZ; >> + else >> + gen = AMD_CHIPSET_NL; >> + } else { >> + gen = AMD_CHIPSET_UNKNOWN; >> + } >> + } >> >> - if (smbus_dev && (smbus_dev->revision < 0x51)) { >> + if ((gen == AMD_CHIPSET_BEFORE_ML) || (gen == AMD_CHIPSET_CZ)) { >> chip->quirks2 |= SDHCI_QUIRK2_CLEAR_TRANSFERMODE_REG_BEFORE_CMD; >> chip->quirks2 |= SDHCI_QUIRK2_BROKEN_HS200; >> } >> diff --git a/include/linux/pci_ids.h b/include/linux/pci_ids.h >> index 2f7b9a4..cb63a7b 100644 >> --- a/include/linux/pci_ids.h >> +++ b/include/linux/pci_ids.h >> @@ -579,6 +579,7 @@ >> #define PCI_DEVICE_ID_AMD_HUDSON2_SATA_IDE 0x7800 >> #define PCI_DEVICE_ID_AMD_HUDSON2_SMBUS 0x780b >> #define PCI_DEVICE_ID_AMD_HUDSON2_IDE 0x780c >> +#define PCI_DEVICE_ID_AMD_KERNCZ_SMBUS 0x790b > > KERNCZ? > > Also, that device id is used only in sdhci-pci.c so it should be defined > there. Only when multiple drivers/compilation units need it, then it > should go to pci_ids.h. Boris, I means I put this KERCZ Mircro in pci_ids.h, and I also will send the other patch to instead the following '0x790b' of codes. I think it is reasonable, right? /* Determine the address of the SMBus areas */ if ((PIIX4_dev->vendor == PCI_VENDOR_ID_AMD && PIIX4_dev->device == PCI_DEVICE_ID_AMD_HUDSON2_SMBUS && PIIX4_dev->revision >= 0x41) || (PIIX4_dev->vendor == PCI_VENDOR_ID_AMD && PIIX4_dev->device == 0x790b && PIIX4_dev->revision >= 0x49)) > > -- > Regards/Gruss, > Boris. > > ECO tip #101: Trim your mails when you reply. > --
On Fri, Jun 05, 2015 at 09:37:09AM +0800, Wan ZongShun wrote: > Boris, I means I put this KERCZ Mircro in pci_ids.h, and I also will > send the other patch to instead the following '0x790b' of codes. I > think it is reasonable, right? No, it means two things: * why does it contain KERNCZ? What does that mean? * #define PCI_DEVICE_ID_AMD_<better_name>_SMBUS 0x790b should be in drivers/mmc/host/sdhci-pci.c as it is used only there. Thanks.
2015-06-05 17:20 GMT+08:00 Borislav Petkov <bp@alien8.de>: > On Fri, Jun 05, 2015 at 09:37:09AM +0800, Wan ZongShun wrote: >> Boris, I means I put this KERCZ Mircro in pci_ids.h, and I also will >> send the other patch to instead the following '0x790b' of codes. I >> think it is reasonable, right? > > No, it means two things: > > * why does it contain KERNCZ? What does that mean? The KERNCZ is new AMD SB/FCH generation name, like HUDSON2 is old FCH generation. We will adopt 0x790b as device ID since from this KERNCZ gereration. > > * #define PCI_DEVICE_ID_AMD_<better_name>_SMBUS 0x790b should be in > drivers/mmc/host/sdhci-pci.c as it is used only there. The i2cpiix4.c driver and eMMC driver both will use this device ID macro, Do you think I should submit two patches synchronously? like patch1: change i2cpiix4 driver to use PCI_DEVICE_ID_AMD_KERNCZ_SMBUS, patch2: for eMMC quirk in sdhci-pci.c? And then the PCI_DEVICE_ID_AMD_KERNCZ_SMBUS can be go into pci ids.h? > > Thanks. > > -- > Regards/Gruss, > Boris. > > ECO tip #101: Trim your mails when you reply. > --
On Fri, Jun 05, 2015 at 06:01:56PM +0800, Wan ZongShun wrote: > The KERNCZ is new AMD SB/FCH generation name, like HUDSON2 is old FCH > generation. > We will adopt 0x790b as device ID since from this KERNCZ gereration. True story. Strange name that. > The i2cpiix4.c driver and eMMC driver both will use this device ID > macro, Do you think I should submit two patches synchronously? like > patch1: change i2cpiix4 driver to use PCI_DEVICE_ID_AMD_KERNCZ_SMBUS, > patch2: for eMMC quirk in sdhci-pci.c? > > And then the PCI_DEVICE_ID_AMD_KERNCZ_SMBUS can be go into pci ids.h? If you have all those patches ready, you could do a first patch adding the PCI ID to pci_ids.h only. Then follow with the different driver changes, i.e. sdhci-pci.c, i2cpiix4, and so on and make it clear in the 0/n message that they all depend on the first one. Or something along those lines. Thanks.
diff --git a/drivers/mmc/host/sdhci-pci.c b/drivers/mmc/host/sdhci-pci.c index f208f20..94f54d2 100644 --- a/drivers/mmc/host/sdhci-pci.c +++ b/drivers/mmc/host/sdhci-pci.c @@ -724,14 +724,37 @@ static const struct sdhci_pci_fixes sdhci_rtsx = { .probe_slot = rtsx_probe_slot, }; +/*AMD chipset generation*/ +enum amd_chipset_gen { + AMD_CHIPSET_BEFORE_ML, + AMD_CHIPSET_CZ, + AMD_CHIPSET_NL, + AMD_CHIPSET_UNKNOWN, +}; + static int amd_probe(struct sdhci_pci_chip *chip) { struct pci_dev *smbus_dev; + enum amd_chipset_gen gen; smbus_dev = pci_get_device(PCI_VENDOR_ID_AMD, PCI_DEVICE_ID_AMD_HUDSON2_SMBUS, NULL); + if (smbus_dev) { + gen = AMD_CHIPSET_BEFORE_ML; + } else { + smbus_dev = pci_get_device(PCI_VENDOR_ID_AMD, + PCI_DEVICE_ID_AMD_KERNCZ_SMBUS, NULL); + if (smbus_dev) { + if (smbus_dev->revision < 0x51) + gen = AMD_CHIPSET_CZ; + else + gen = AMD_CHIPSET_NL; + } else { + gen = AMD_CHIPSET_UNKNOWN; + } + } - if (smbus_dev && (smbus_dev->revision < 0x51)) { + if ((gen == AMD_CHIPSET_BEFORE_ML) || (gen == AMD_CHIPSET_CZ)) { chip->quirks2 |= SDHCI_QUIRK2_CLEAR_TRANSFERMODE_REG_BEFORE_CMD; chip->quirks2 |= SDHCI_QUIRK2_BROKEN_HS200; } diff --git a/include/linux/pci_ids.h b/include/linux/pci_ids.h index 2f7b9a4..cb63a7b 100644 --- a/include/linux/pci_ids.h +++ b/include/linux/pci_ids.h @@ -579,6 +579,7 @@ #define PCI_DEVICE_ID_AMD_HUDSON2_SATA_IDE 0x7800 #define PCI_DEVICE_ID_AMD_HUDSON2_SMBUS 0x780b #define PCI_DEVICE_ID_AMD_HUDSON2_IDE 0x780c +#define PCI_DEVICE_ID_AMD_KERNCZ_SMBUS 0x790b #define PCI_VENDOR_ID_TRIDENT 0x1023 #define PCI_DEVICE_ID_TRIDENT_4DWAVE_DX 0x2000