* [PATCH] ata: ahci: Add initialization quirk for JMicron JMB585/JMB582 controllers
@ 2022-09-15 0:11 MD Lin
2022-09-16 14:35 ` Damien Le Moal
0 siblings, 1 reply; 2+ messages in thread
From: MD Lin @ 2022-09-15 0:11 UTC (permalink / raw)
To: damien.lemoal
Cc: linux-ide, linux-kernel, mdlin, kevinliu, charonchen, corahuang,
mhchen, georgechao, banks, tzuwei
JMicron JMB585/JMB582 does not enable specific error bit handling functions
by default so this patch enable these functions for better compatibility.
Besides, these patches also adjust SATA RX/TX_GEN1/TX_GEN2 parameters for
better compatibility. These patches had been tested in JMicron Test
Laboratory and been implemented to our customers.
Signed-off-by: MD Lin <mdlin@jmicron.com>
---
drivers/ata/ahci.c | 71 ++++++++++++++++++++++++++++++++++++++++++++++
drivers/ata/ahci.h | 23 +++++++++++++++
2 files changed, 94 insertions(+)
mode change 100755 => 100644 drivers/ata/ahci.h
diff --git a/drivers/ata/ahci.c b/drivers/ata/ahci.c
index 505920d45..3e9e3b8f8 100644
--- a/drivers/ata/ahci.c
+++ b/drivers/ata/ahci.c
@@ -1657,6 +1657,75 @@ static void ahci_intel_pcs_quirk(struct pci_dev *pdev, struct ahci_host_priv *hp
}
}
+static void ahci_jmb58x_write_sata(void __iomem *mmio, u32 addr, u32 data)
+{
+ writel((addr & 0x01FFFUL) + (1UL << 18UL), mmio + 0xC0);
+ writel(data, mmio + 0xC8);
+}
+
+static void ahci_jmb58x_quirk(void __iomem *mmio)
+{
+ u32 pi = readl(mmio + HOST_PORTS_IMPL);
+ u32 b8_data;
+
+ /*
+ * JMB582: PI is 0x03
+ * JMB585: PI is 0x1f
+ */
+
+ /*
+ * enable error bit handling functions, these might overwrite
+ * the setting which loads from external SPI flash.
+ * the address and value are defined in adapter specs.
+ */
+ b8_data = (pi > 3) ? 0x13 : 0x92;
+ writel(JMB58X_EH_MODIFY_ON + b8_data, mmio + 0xB8);
+ writel(JMB58X_EH_GENERAL, mmio + 0x30);
+ writel(JMB58X_EH_CFIS_RETRY, mmio + 0x34);
+ writel(JMB58X_EH_DROP_D2H, mmio + 0x38);
+ writel(JMB58X_EH_MODIFY_OFF + b8_data, mmio + 0xB8);
+ writel(JMB58X_EH_TX_LOCK, mmio + 0xB0);
+
+ /*
+ * set SATA configuration, these might overwrite
+ * the setting which loads from external SPI flash.
+ * the address and value are defined in adapter specs.
+ */
+ ahci_jmb58x_write_sata(mmio, 0x06, JMB58X_SATA0_RX);
+ ahci_jmb58x_write_sata(mmio, 0x13, JMB58X_SATA1_RX);
+ ahci_jmb58x_write_sata(mmio, 0x73, JMB58X_SATA0_TX_GEN2);
+ ahci_jmb58x_write_sata(mmio, 0x75, JMB58X_SATA1_TX_GEN2);
+ ahci_jmb58x_write_sata(mmio, 0x74, JMB58X_SATA0_TX_GEN1);
+ ahci_jmb58x_write_sata(mmio, 0x80, JMB58X_SATA1_TX_GEN1);
+ if (pi > 3) {
+ ahci_jmb58x_write_sata(mmio, 0x20, JMB58X_SATA2_RX);
+ ahci_jmb58x_write_sata(mmio, 0x2D, JMB58X_SATA3_RX);
+ ahci_jmb58x_write_sata(mmio, 0x3A, JMB58X_SATA4_RX);
+ ahci_jmb58x_write_sata(mmio, 0x79, JMB58X_SATA3_TX_GEN2);
+ ahci_jmb58x_write_sata(mmio, 0x83, JMB58X_SATA3_TX_GEN2_EXT);
+ ahci_jmb58x_write_sata(mmio, 0x7A, JMB58X_SATA3_TX_GEN1);
+ ahci_jmb58x_write_sata(mmio, 0x84, JMB58X_SATA3_TX_GEN1_EXT);
+ }
+}
+
+static void ahci_jmicron_quirk(struct pci_dev *pdev,
+ struct ahci_host_priv *hpriv)
+{
+ void __iomem *mmio = hpriv->mmio;
+ u8 tmp8;
+
+ if (pdev->vendor != PCI_VENDOR_ID_JMICRON)
+ return;
+
+ switch (pdev->device) {
+ case 0x585:
+ tmp8 = readb(mmio + 0x44);
+ if (tmp8) /* check controller version */
+ ahci_jmb58x_quirk(mmio);
+ break;
+ }
+}
+
static int ahci_init_one(struct pci_dev *pdev, const struct pci_device_id *ent)
{
unsigned int board_id = ent->driver_data;
@@ -1775,6 +1844,8 @@ static int ahci_init_one(struct pci_dev *pdev, const struct pci_device_id *ent)
*/
ahci_intel_pcs_quirk(pdev, hpriv);
+ ahci_jmicron_quirk(pdev, hpriv);
+
/* prepare host */
if (hpriv->cap & HOST_CAP_NCQ) {
pi.flags |= ATA_FLAG_NCQ;
diff --git a/drivers/ata/ahci.h b/drivers/ata/ahci.h
old mode 100755
new mode 100644
index 9290e787a..82ecc6f2c
--- a/drivers/ata/ahci.h
+++ b/drivers/ata/ahci.h
@@ -52,6 +52,29 @@
#define EM_MSG_LED_VALUE_OFF 0xfff80000
#define EM_MSG_LED_VALUE_ON 0x00010000
+/* JMicron JMB585/JMB582 Error Bit Handling Register Value */
+#define JMB58X_EH_MODIFY_ON 0x03060004
+#define JMB58X_EH_MODIFY_OFF 0x03060000
+#define JMB58X_EH_GENERAL 0x00FF0B01
+#define JMB58X_EH_CFIS_RETRY 0x0000003F
+#define JMB58X_EH_DROP_D2H 0x0000001F
+#define JMB58X_EH_TX_LOCK 0xF9E4EFBF
+
+/* JMicron JMB585/JMB582 SATA PHY Register Value */
+#define JMB58X_SATA0_RX 0x70005BE3
+#define JMB58X_SATA1_RX 0x70005BE3
+#define JMB58X_SATA2_RX 0x70005BE3
+#define JMB58X_SATA3_RX 0x70005BE3
+#define JMB58X_SATA4_RX 0x70005BE3
+#define JMB58X_SATA0_TX_GEN1 0x00000024
+#define JMB58X_SATA1_TX_GEN1 0x250B0003
+#define JMB58X_SATA3_TX_GEN1 0x00000024
+#define JMB58X_SATA3_TX_GEN1_EXT 0x250B0003
+#define JMB58X_SATA0_TX_GEN2 0x000001E5
+#define JMB58X_SATA1_TX_GEN2 0x000001E5
+#define JMB58X_SATA3_TX_GEN2 0x000001E5
+#define JMB58X_SATA3_TX_GEN2_EXT 0x250B0003
+
enum {
AHCI_MAX_PORTS = 32,
AHCI_MAX_CLKS = 5,
--
2.17.1
^ permalink raw reply related [flat|nested] 2+ messages in thread
* Re: [PATCH] ata: ahci: Add initialization quirk for JMicron JMB585/JMB582 controllers
2022-09-15 0:11 [PATCH] ata: ahci: Add initialization quirk for JMicron JMB585/JMB582 controllers MD Lin
@ 2022-09-16 14:35 ` Damien Le Moal
0 siblings, 0 replies; 2+ messages in thread
From: Damien Le Moal @ 2022-09-16 14:35 UTC (permalink / raw)
To: MD Lin
Cc: linux-ide, linux-kernel, kevinliu, charonchen, corahuang, mhchen,
georgechao, banks, tzuwei
On 2022/09/15 1:11, MD Lin wrote:
> JMicron JMB585/JMB582 does not enable specific error bit handling functions
> by default so this patch enable these functions for better compatibility.
> Besides, these patches also adjust SATA RX/TX_GEN1/TX_GEN2 parameters for
> better compatibility. These patches had been tested in JMicron Test
> Laboratory and been implemented to our customers.
>
> Signed-off-by: MD Lin <mdlin@jmicron.com>
> ---
> drivers/ata/ahci.c | 71 ++++++++++++++++++++++++++++++++++++++++++++++
> drivers/ata/ahci.h | 23 +++++++++++++++
> 2 files changed, 94 insertions(+)
> mode change 100755 => 100644 drivers/ata/ahci.h
>
> diff --git a/drivers/ata/ahci.c b/drivers/ata/ahci.c
> index 505920d45..3e9e3b8f8 100644
> --- a/drivers/ata/ahci.c
> +++ b/drivers/ata/ahci.c
> @@ -1657,6 +1657,75 @@ static void ahci_intel_pcs_quirk(struct pci_dev *pdev, struct ahci_host_priv *hp
> }
> }
>
> +static void ahci_jmb58x_write_sata(void __iomem *mmio, u32 addr, u32 data)
> +{
> + writel((addr & 0x01FFFUL) + (1UL << 18UL), mmio + 0xC0);
> + writel(data, mmio + 0xC8);
> +}
> +
> +static void ahci_jmb58x_quirk(void __iomem *mmio)
> +{
> + u32 pi = readl(mmio + HOST_PORTS_IMPL);
> + u32 b8_data;
> +
> + /*
> + * JMB582: PI is 0x03
> + * JMB585: PI is 0x1f
> + */
What is this comment for ?
> +
> + /*
> + * enable error bit handling functions, these might overwrite
> + * the setting which loads from external SPI flash.
> + * the address and value are defined in adapter specs.
> + */
> + b8_data = (pi > 3) ? 0x13 : 0x92;
This looks strange. If pi is fixed depending on the controller type, why not use
a switch-case here with the values in the comments above defined as macros ?
Something like:
switch (pi) {
case JMB582:
b8_data = 0x92;
case JMB585:
default:
b8_data = 0x13;
}
This is a lot more readable.
> + writel(JMB58X_EH_MODIFY_ON + b8_data, mmio + 0xB8);
> + writel(JMB58X_EH_GENERAL, mmio + 0x30);
> + writel(JMB58X_EH_CFIS_RETRY, mmio + 0x34);
> + writel(JMB58X_EH_DROP_D2H, mmio + 0x38);
> + writel(JMB58X_EH_MODIFY_OFF + b8_data, mmio + 0xB8);
> + writel(JMB58X_EH_TX_LOCK, mmio + 0xB0);
Why not define all these magic values as macros using the register names ?
Anyone comparing the code to the controller specs will more easily understand.
> +
> + /*
> + * set SATA configuration, these might overwrite
> + * the setting which loads from external SPI flash.
> + * the address and value are defined in adapter specs.
> + */
> + ahci_jmb58x_write_sata(mmio, 0x06, JMB58X_SATA0_RX);
> + ahci_jmb58x_write_sata(mmio, 0x13, JMB58X_SATA1_RX);
> + ahci_jmb58x_write_sata(mmio, 0x73, JMB58X_SATA0_TX_GEN2);
> + ahci_jmb58x_write_sata(mmio, 0x75, JMB58X_SATA1_TX_GEN2);
> + ahci_jmb58x_write_sata(mmio, 0x74, JMB58X_SATA0_TX_GEN1);
> + ahci_jmb58x_write_sata(mmio, 0x80, JMB58X_SATA1_TX_GEN1);
> + if (pi > 3) {
> + ahci_jmb58x_write_sata(mmio, 0x20, JMB58X_SATA2_RX);
> + ahci_jmb58x_write_sata(mmio, 0x2D, JMB58X_SATA3_RX);
> + ahci_jmb58x_write_sata(mmio, 0x3A, JMB58X_SATA4_RX);
> + ahci_jmb58x_write_sata(mmio, 0x79, JMB58X_SATA3_TX_GEN2);
> + ahci_jmb58x_write_sata(mmio, 0x83, JMB58X_SATA3_TX_GEN2_EXT);
> + ahci_jmb58x_write_sata(mmio, 0x7A, JMB58X_SATA3_TX_GEN1);
> + ahci_jmb58x_write_sata(mmio, 0x84, JMB58X_SATA3_TX_GEN1_EXT);
> + }
Same comment here.
> +}
> +
> +static void ahci_jmicron_quirk(struct pci_dev *pdev,
> + struct ahci_host_priv *hpriv)
> +{
> + void __iomem *mmio = hpriv->mmio;
> + u8 tmp8;
> +
> + if (pdev->vendor != PCI_VENDOR_ID_JMICRON)
> + return;
> +
> + switch (pdev->device) {
> + case 0x585:
> + tmp8 = readb(mmio + 0x44);
> + if (tmp8) /* check controller version */
> + ahci_jmb58x_quirk(mmio);
The tmp8 variable is not needed:
if (readb(mmio + 0x44))
ahci_jmb58x_quirk(mmio);
is fine, with the magic value 0x44 defined as a macro with a descriptive name.
> + break;
> + }
> +}
> +
> static int ahci_init_one(struct pci_dev *pdev, const struct pci_device_id *ent)
> {
> unsigned int board_id = ent->driver_data;
> @@ -1775,6 +1844,8 @@ static int ahci_init_one(struct pci_dev *pdev, const struct pci_device_id *ent)
> */
> ahci_intel_pcs_quirk(pdev, hpriv);
>
> + ahci_jmicron_quirk(pdev, hpriv);
> +
> /* prepare host */
> if (hpriv->cap & HOST_CAP_NCQ) {
> pi.flags |= ATA_FLAG_NCQ;
> diff --git a/drivers/ata/ahci.h b/drivers/ata/ahci.h
> old mode 100755
> new mode 100644
> index 9290e787a..82ecc6f2c
> --- a/drivers/ata/ahci.h
> +++ b/drivers/ata/ahci.h
> @@ -52,6 +52,29 @@
> #define EM_MSG_LED_VALUE_OFF 0xfff80000
> #define EM_MSG_LED_VALUE_ON 0x00010000
>
> +/* JMicron JMB585/JMB582 Error Bit Handling Register Value */
> +#define JMB58X_EH_MODIFY_ON 0x03060004
> +#define JMB58X_EH_MODIFY_OFF 0x03060000
> +#define JMB58X_EH_GENERAL 0x00FF0B01
> +#define JMB58X_EH_CFIS_RETRY 0x0000003F
> +#define JMB58X_EH_DROP_D2H 0x0000001F
> +#define JMB58X_EH_TX_LOCK 0xF9E4EFBF
> +
> +/* JMicron JMB585/JMB582 SATA PHY Register Value */
> +#define JMB58X_SATA0_RX 0x70005BE3
> +#define JMB58X_SATA1_RX 0x70005BE3
> +#define JMB58X_SATA2_RX 0x70005BE3
> +#define JMB58X_SATA3_RX 0x70005BE3
> +#define JMB58X_SATA4_RX 0x70005BE3
> +#define JMB58X_SATA0_TX_GEN1 0x00000024
> +#define JMB58X_SATA1_TX_GEN1 0x250B0003
> +#define JMB58X_SATA3_TX_GEN1 0x00000024
> +#define JMB58X_SATA3_TX_GEN1_EXT 0x250B0003
> +#define JMB58X_SATA0_TX_GEN2 0x000001E5
> +#define JMB58X_SATA1_TX_GEN2 0x000001E5
> +#define JMB58X_SATA3_TX_GEN2 0x000001E5
> +#define JMB58X_SATA3_TX_GEN2_EXT 0x250B0003
> +
> enum {
> AHCI_MAX_PORTS = 32,
> AHCI_MAX_CLKS = 5,
--
Damien Le Moal
Western Digital Research
^ permalink raw reply [flat|nested] 2+ messages in thread
end of thread, other threads:[~2022-09-16 14:35 UTC | newest]
Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-09-15 0:11 [PATCH] ata: ahci: Add initialization quirk for JMicron JMB585/JMB582 controllers MD Lin
2022-09-16 14:35 ` Damien Le Moal
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).