2020-04-24 11:39:18

by Jose Abreu

[permalink] [raw]
Subject: [PATCH 3/5] scsi: ufs: tc-dwc-pci: Use PDI ID to match Test Chip type

In preparation for the addition of new Test Chips, we re-arrange the
initialization sequence so that we rely on PCI ID to match for given
Test Chip type.

Signed-off-by: Jose Abreu <[email protected]>

---
Cc: Joao Lima <[email protected]>
Cc: Jose Abreu <[email protected]>
Cc: Alim Akhtar <[email protected]>
Cc: Avri Altman <[email protected]>
Cc: "James E.J. Bottomley" <[email protected]>
Cc: "Martin K. Petersen" <[email protected]>
Cc: [email protected]
Cc: [email protected]
---
drivers/scsi/ufs/tc-dwc-pci.c | 68 ++++++++++++++++++++++++++++---------------
1 file changed, 44 insertions(+), 24 deletions(-)

diff --git a/drivers/scsi/ufs/tc-dwc-pci.c b/drivers/scsi/ufs/tc-dwc-pci.c
index aeb11f7f0c91..74a2d80d32bd 100644
--- a/drivers/scsi/ufs/tc-dwc-pci.c
+++ b/drivers/scsi/ufs/tc-dwc-pci.c
@@ -14,6 +14,11 @@
#include <linux/pci.h>
#include <linux/pm_runtime.h>

+struct tc_dwc_data {
+ struct ufs_hba_variant_ops ops;
+ int (*setup)(struct pci_dev *pdev, struct tc_dwc_data *data);
+};
+
/* Test Chip type expected values */
#define TC_G210_20BIT 20
#define TC_G210_40BIT 40
@@ -23,6 +28,20 @@ static int tc_type = TC_G210_INV;
module_param(tc_type, int, 0);
MODULE_PARM_DESC(tc_type, "Test Chip Type (20 = 20-bit, 40 = 40-bit)");

+static int tc_dwc_g210_set_config(struct pci_dev *pdev, struct tc_dwc_data *data)
+{
+ if (tc_type == TC_G210_20BIT) {
+ data->ops.phy_initialization = tc_dwc_g210_config_20_bit;
+ } else if (tc_type == TC_G210_40BIT) {
+ data->ops.phy_initialization = tc_dwc_g210_config_40_bit;
+ } else {
+ dev_err(&pdev->dev, "test chip version not specified\n");
+ return -EPERM;
+ }
+
+ return 0;
+}
+
static int tc_dwc_pci_suspend(struct device *dev)
{
return ufshcd_system_suspend(dev_get_drvdata(dev));
@@ -48,14 +67,6 @@ static int tc_dwc_pci_runtime_idle(struct device *dev)
return ufshcd_runtime_idle(dev_get_drvdata(dev));
}

-/*
- * struct ufs_hba_dwc_vops - UFS DWC specific variant operations
- */
-static struct ufs_hba_variant_ops tc_dwc_pci_hba_vops = {
- .name = "tc-dwc-pci",
- .link_startup_notify = ufshcd_dwc_link_startup_notify,
-};
-
/**
* tc_dwc_pci_shutdown - main function to put the controller in reset state
* @pdev: pointer to PCI device handle
@@ -89,22 +100,11 @@ static void tc_dwc_pci_remove(struct pci_dev *pdev)
static int
tc_dwc_pci_probe(struct pci_dev *pdev, const struct pci_device_id *id)
{
- struct ufs_hba *hba;
+ struct tc_dwc_data *data = (struct tc_dwc_data *)id->driver_data;
void __iomem *mmio_base;
+ struct ufs_hba *hba;
int err;

- /* Check Test Chip type and set the specific setup routine */
- if (tc_type == TC_G210_20BIT) {
- tc_dwc_pci_hba_vops.phy_initialization =
- tc_dwc_g210_config_20_bit;
- } else if (tc_type == TC_G210_40BIT) {
- tc_dwc_pci_hba_vops.phy_initialization =
- tc_dwc_g210_config_40_bit;
- } else {
- dev_err(&pdev->dev, "test chip version not specified\n");
- return -EPERM;
- }
-
err = pcim_enable_device(pdev);
if (err) {
dev_err(&pdev->dev, "pcim_enable_device failed\n");
@@ -127,7 +127,16 @@ tc_dwc_pci_probe(struct pci_dev *pdev, const struct pci_device_id *id)
return err;
}

- hba->vops = &tc_dwc_pci_hba_vops;
+ /* Check Test Chip type and set the specific setup routine */
+ if (data && data->setup) {
+ err = data->setup(pdev, data);
+ if (err)
+ return err;
+ } else {
+ return -ENOENT;
+ }
+
+ hba->vops = &data->ops;

err = ufshcd_init(hba, mmio_base, pdev->irq);
if (err) {
@@ -150,9 +159,20 @@ static const struct dev_pm_ops tc_dwc_pci_pm_ops = {
.runtime_idle = tc_dwc_pci_runtime_idle,
};

+static struct tc_dwc_data tc_dwc_g210_data = {
+ .setup = tc_dwc_g210_set_config,
+ .ops = {
+ .name = "tc-dwc-g210-pci",
+ .link_startup_notify = ufshcd_dwc_link_startup_notify,
+ },
+};
+
+#define PCI_DEVICE_ID_SYNOPSYS_TC_G210_1 0xB101
+#define PCI_DEVICE_ID_SYNOPSYS_TC_G210_2 0xB102
+
static const struct pci_device_id tc_dwc_pci_tbl[] = {
- { PCI_VENDOR_ID_SYNOPSYS, 0xB101, PCI_ANY_ID, PCI_ANY_ID, 0, 0, 0 },
- { PCI_VENDOR_ID_SYNOPSYS, 0xB102, PCI_ANY_ID, PCI_ANY_ID, 0, 0, 0 },
+ { PCI_DEVICE_DATA(SYNOPSYS, TC_G210_1, &tc_dwc_g210_data) },
+ { PCI_DEVICE_DATA(SYNOPSYS, TC_G210_2, &tc_dwc_g210_data) },
{ } /* terminate list */
};

--
2.7.4


2020-04-24 12:05:45

by Winkler, Tomas

[permalink] [raw]
Subject: RE: [PATCH 3/5] scsi: ufs: tc-dwc-pci: Use PDI ID to match Test Chip type

>
> In preparation for the addition of new Test Chips, we re-arrange the
> initialization sequence so that we rely on PCI ID to match for given Test Chip
> type.
>
> Signed-off-by: Jose Abreu <[email protected]>
>
> ---
> Cc: Joao Lima <[email protected]>
> Cc: Jose Abreu <[email protected]>
> Cc: Alim Akhtar <[email protected]>
> Cc: Avri Altman <[email protected]>
> Cc: "James E.J. Bottomley" <[email protected]>
> Cc: "Martin K. Petersen" <[email protected]>
> Cc: [email protected]
> Cc: [email protected]
> ---
> drivers/scsi/ufs/tc-dwc-pci.c | 68 ++++++++++++++++++++++++++++-----------
> ----
> 1 file changed, 44 insertions(+), 24 deletions(-)
>
> diff --git a/drivers/scsi/ufs/tc-dwc-pci.c b/drivers/scsi/ufs/tc-dwc-pci.c index
> aeb11f7f0c91..74a2d80d32bd 100644
> --- a/drivers/scsi/ufs/tc-dwc-pci.c
> +++ b/drivers/scsi/ufs/tc-dwc-pci.c
> @@ -14,6 +14,11 @@
> #include <linux/pci.h>
> #include <linux/pm_runtime.h>
>
> +struct tc_dwc_data {
> + struct ufs_hba_variant_ops ops;
> + int (*setup)(struct pci_dev *pdev, struct tc_dwc_data *data); };
> +
> /* Test Chip type expected values */
> #define TC_G210_20BIT 20
> #define TC_G210_40BIT 40
> @@ -23,6 +28,20 @@ static int tc_type = TC_G210_INV;
> module_param(tc_type, int, 0); MODULE_PARM_DESC(tc_type, "Test Chip
> Type (20 = 20-bit, 40 = 40-bit)");
>
> +static int tc_dwc_g210_set_config(struct pci_dev *pdev, struct
> +tc_dwc_data *data) {
> + if (tc_type == TC_G210_20BIT) {
> + data->ops.phy_initialization = tc_dwc_g210_config_20_bit;
> + } else if (tc_type == TC_G210_40BIT) {
> + data->ops.phy_initialization = tc_dwc_g210_config_40_bit;
> + } else {
> + dev_err(&pdev->dev, "test chip version not specified\n");
> + return -EPERM;
> + }
> +
> + return 0;
> +}
> +
> static int tc_dwc_pci_suspend(struct device *dev) {
> return ufshcd_system_suspend(dev_get_drvdata(dev));
> @@ -48,14 +67,6 @@ static int tc_dwc_pci_runtime_idle(struct device *dev)
> return ufshcd_runtime_idle(dev_get_drvdata(dev));
> }
>
> -/*
> - * struct ufs_hba_dwc_vops - UFS DWC specific variant operations
> - */
> -static struct ufs_hba_variant_ops tc_dwc_pci_hba_vops = {
> - .name = "tc-dwc-pci",
> - .link_startup_notify = ufshcd_dwc_link_startup_notify,
> -};
> -
> /**
> * tc_dwc_pci_shutdown - main function to put the controller in reset state
> * @pdev: pointer to PCI device handle
> @@ -89,22 +100,11 @@ static void tc_dwc_pci_remove(struct pci_dev *pdev)
> static int tc_dwc_pci_probe(struct pci_dev *pdev, const struct pci_device_id
> *id) {
> - struct ufs_hba *hba;
> + struct tc_dwc_data *data = (struct tc_dwc_data *)id->driver_data;
> void __iomem *mmio_base;
> + struct ufs_hba *hba;
> int err;
>
> - /* Check Test Chip type and set the specific setup routine */
> - if (tc_type == TC_G210_20BIT) {
> - tc_dwc_pci_hba_vops.phy_initialization =
> - tc_dwc_g210_config_20_bit;
> - } else if (tc_type == TC_G210_40BIT) {
> - tc_dwc_pci_hba_vops.phy_initialization =
> - tc_dwc_g210_config_40_bit;
> - } else {
> - dev_err(&pdev->dev, "test chip version not specified\n");
> - return -EPERM;
> - }
> -
> err = pcim_enable_device(pdev);
> if (err) {
> dev_err(&pdev->dev, "pcim_enable_device failed\n"); @@ -
> 127,7 +127,16 @@ tc_dwc_pci_probe(struct pci_dev *pdev, const struct
> pci_device_id *id)
> return err;
> }
>
> - hba->vops = &tc_dwc_pci_hba_vops;
> + /* Check Test Chip type and set the specific setup routine */
> + if (data && data->setup) {
> + err = data->setup(pdev, data);
> + if (err)
> + return err;
> + } else {
> + return -ENOENT;
> + }
> +
> + hba->vops = &data->ops;
>
> err = ufshcd_init(hba, mmio_base, pdev->irq);
> if (err) {
> @@ -150,9 +159,20 @@ static const struct dev_pm_ops tc_dwc_pci_pm_ops
> = {
> .runtime_idle = tc_dwc_pci_runtime_idle,
> };
>
> +static struct tc_dwc_data tc_dwc_g210_data = {

Constify the struct, if possible.

> + .setup = tc_dwc_g210_set_config,
> + .ops = {
> + .name = "tc-dwc-g210-pci",
> + .link_startup_notify = ufshcd_dwc_link_startup_notify,
> + },
> +};
> +
> +#define PCI_DEVICE_ID_SYNOPSYS_TC_G210_1 0xB101
> +#define PCI_DEVICE_ID_SYNOPSYS_TC_G210_2 0xB102
> +
> static const struct pci_device_id tc_dwc_pci_tbl[] = {
> - { PCI_VENDOR_ID_SYNOPSYS, 0xB101, PCI_ANY_ID, PCI_ANY_ID, 0, 0,
> 0 },
> - { PCI_VENDOR_ID_SYNOPSYS, 0xB102, PCI_ANY_ID, PCI_ANY_ID, 0, 0,
> 0 },
> + { PCI_DEVICE_DATA(SYNOPSYS, TC_G210_1, &tc_dwc_g210_data) },
> + { PCI_DEVICE_DATA(SYNOPSYS, TC_G210_2, &tc_dwc_g210_data) },
> { } /* terminate list */
> };
>
> --
> 2.7.4

2020-04-24 12:10:49

by Jose Abreu

[permalink] [raw]
Subject: RE: [PATCH 3/5] scsi: ufs: tc-dwc-pci: Use PDI ID to match Test Chip type

From: Winkler, Tomas <[email protected]>
Date: Apr/24/2020, 13:00:33 (UTC+00:00)

> >
> > In preparation for the addition of new Test Chips, we re-arrange the
> > initialization sequence so that we rely on PCI ID to match for given Test Chip
> > type.
> >
> > Signed-off-by: Jose Abreu <[email protected]>
> >
> > ---
> > Cc: Joao Lima <[email protected]>
> > Cc: Jose Abreu <[email protected]>
> > Cc: Alim Akhtar <[email protected]>
> > Cc: Avri Altman <[email protected]>
> > Cc: "James E.J. Bottomley" <[email protected]>
> > Cc: "Martin K. Petersen" <[email protected]>
> > Cc: [email protected]
> > Cc: [email protected]
> > ---
> > drivers/scsi/ufs/tc-dwc-pci.c | 68 ++++++++++++++++++++++++++++-----------
> > ----
> > 1 file changed, 44 insertions(+), 24 deletions(-)
> >
> > diff --git a/drivers/scsi/ufs/tc-dwc-pci.c b/drivers/scsi/ufs/tc-dwc-pci.c index
> > aeb11f7f0c91..74a2d80d32bd 100644
> > --- a/drivers/scsi/ufs/tc-dwc-pci.c
> > +++ b/drivers/scsi/ufs/tc-dwc-pci.c
> > @@ -14,6 +14,11 @@
> > #include <linux/pci.h>
> > #include <linux/pm_runtime.h>
> >
> > +struct tc_dwc_data {
> > + struct ufs_hba_variant_ops ops;
> > + int (*setup)(struct pci_dev *pdev, struct tc_dwc_data *data); };
> > +
> > /* Test Chip type expected values */
> > #define TC_G210_20BIT 20
> > #define TC_G210_40BIT 40
> > @@ -23,6 +28,20 @@ static int tc_type = TC_G210_INV;
> > module_param(tc_type, int, 0); MODULE_PARM_DESC(tc_type, "Test Chip
> > Type (20 = 20-bit, 40 = 40-bit)");
> >
> > +static int tc_dwc_g210_set_config(struct pci_dev *pdev, struct
> > +tc_dwc_data *data) {
> > + if (tc_type == TC_G210_20BIT) {
> > + data->ops.phy_initialization = tc_dwc_g210_config_20_bit;
> > + } else if (tc_type == TC_G210_40BIT) {
> > + data->ops.phy_initialization = tc_dwc_g210_config_40_bit;
> > + } else {
> > + dev_err(&pdev->dev, "test chip version not specified\n");
> > + return -EPERM;
> > + }
> > +
> > + return 0;
> > +}
> > +
> > static int tc_dwc_pci_suspend(struct device *dev) {
> > return ufshcd_system_suspend(dev_get_drvdata(dev));
> > @@ -48,14 +67,6 @@ static int tc_dwc_pci_runtime_idle(struct device *dev)
> > return ufshcd_runtime_idle(dev_get_drvdata(dev));
> > }
> >
> > -/*
> > - * struct ufs_hba_dwc_vops - UFS DWC specific variant operations
> > - */
> > -static struct ufs_hba_variant_ops tc_dwc_pci_hba_vops = {
> > - .name = "tc-dwc-pci",
> > - .link_startup_notify = ufshcd_dwc_link_startup_notify,
> > -};
> > -
> > /**
> > * tc_dwc_pci_shutdown - main function to put the controller in reset state
> > * @pdev: pointer to PCI device handle
> > @@ -89,22 +100,11 @@ static void tc_dwc_pci_remove(struct pci_dev *pdev)
> > static int tc_dwc_pci_probe(struct pci_dev *pdev, const struct pci_device_id
> > *id) {
> > - struct ufs_hba *hba;
> > + struct tc_dwc_data *data = (struct tc_dwc_data *)id->driver_data;
> > void __iomem *mmio_base;
> > + struct ufs_hba *hba;
> > int err;
> >
> > - /* Check Test Chip type and set the specific setup routine */
> > - if (tc_type == TC_G210_20BIT) {
> > - tc_dwc_pci_hba_vops.phy_initialization =
> > - tc_dwc_g210_config_20_bit;
> > - } else if (tc_type == TC_G210_40BIT) {
> > - tc_dwc_pci_hba_vops.phy_initialization =
> > - tc_dwc_g210_config_40_bit;
> > - } else {
> > - dev_err(&pdev->dev, "test chip version not specified\n");
> > - return -EPERM;
> > - }
> > -
> > err = pcim_enable_device(pdev);
> > if (err) {
> > dev_err(&pdev->dev, "pcim_enable_device failed\n"); @@ -
> > 127,7 +127,16 @@ tc_dwc_pci_probe(struct pci_dev *pdev, const struct
> > pci_device_id *id)
> > return err;
> > }
> >
> > - hba->vops = &tc_dwc_pci_hba_vops;
> > + /* Check Test Chip type and set the specific setup routine */
> > + if (data && data->setup) {
> > + err = data->setup(pdev, data);
> > + if (err)
> > + return err;
> > + } else {
> > + return -ENOENT;
> > + }
> > +
> > + hba->vops = &data->ops;
> >
> > err = ufshcd_init(hba, mmio_base, pdev->irq);
> > if (err) {
> > @@ -150,9 +159,20 @@ static const struct dev_pm_ops tc_dwc_pci_pm_ops
> > = {
> > .runtime_idle = tc_dwc_pci_runtime_idle,
> > };
> >
> > +static struct tc_dwc_data tc_dwc_g210_data = {
>
> Constify the struct, if possible.

Actually, I can't because I overwrite one of the ops callback in the
probe() depending on TC specified.

---
Thanks,
Jose Miguel Abreu