| Message ID | 1377577093-10068-2-git-send-email-sthumma@codeaurora.org (mailing list archive) |
|---|---|
| State | Not Applicable, archived |
| Headers | show |
Looks good to me. Reviewed-by: Subhash Jadavani <subhashj@codeaurora.org> On 8/27/2013 9:48 AM, Sujit Reddy Thumma wrote: > Some vendor specific controller versions might need to configure > vendor specific - registers, clocks, voltage regulators etc. to > initialize the host controller UTP layer and Uni-Pro stack. > Provide some common initialization operations that can be used > to configure vendor specifics. The methods can be extended in > future, for example, for power mode transitions. > > The operations are vendor/board specific and hence determined with > the help of compatible property in device tree. > > Signed-off-by: Sujit Reddy Thumma <sthumma@codeaurora.org> > --- > drivers/scsi/ufs/ufshcd-pci.c | 8 +- > drivers/scsi/ufs/ufshcd-pltfrm.c | 24 +++++- > drivers/scsi/ufs/ufshcd.c | 157 +++++++++++++++++++++++++++++++-------- > drivers/scsi/ufs/ufshcd.h | 34 ++++++++- > 4 files changed, 187 insertions(+), 36 deletions(-) > > diff --git a/drivers/scsi/ufs/ufshcd-pci.c b/drivers/scsi/ufs/ufshcd-pci.c > index a823cf4..6b0d299 100644 > --- a/drivers/scsi/ufs/ufshcd-pci.c > +++ b/drivers/scsi/ufs/ufshcd-pci.c > @@ -191,7 +191,13 @@ ufshcd_pci_probe(struct pci_dev *pdev, const struct pci_device_id *id) > return err; > } > > - err = ufshcd_init(&pdev->dev, &hba, mmio_base, pdev->irq); > + err = ufshcd_alloc_host(&pdev->dev, &hba); > + if (err) { > + dev_err(&pdev->dev, "Allocation failed\n"); > + return err; > + } > + > + err = ufshcd_init(hba, mmio_base, pdev->irq); > if (err) { > dev_err(&pdev->dev, "Initialization failed\n"); > return err; > diff --git a/drivers/scsi/ufs/ufshcd-pltfrm.c b/drivers/scsi/ufs/ufshcd-pltfrm.c > index 5e46232..9c94052 100644 > --- a/drivers/scsi/ufs/ufshcd-pltfrm.c > +++ b/drivers/scsi/ufs/ufshcd-pltfrm.c > @@ -35,9 +35,23 @@ > > #include <linux/platform_device.h> > #include <linux/pm_runtime.h> > +#include <linux/of.h> > > #include "ufshcd.h" > > +static const struct of_device_id ufs_of_match[]; > +static struct ufs_hba_variant_ops *get_variant_ops(struct device *dev) > +{ > + if (dev->of_node) { > + const struct of_device_id *match; > + match = of_match_node(ufs_of_match, dev->of_node); > + if (match) > + return (struct ufs_hba_variant_ops *)match->data; > + } > + > + return NULL; > +} > + > #ifdef CONFIG_PM > /** > * ufshcd_pltfrm_suspend - suspend power management function > @@ -150,10 +164,18 @@ static int ufshcd_pltfrm_probe(struct platform_device *pdev) > goto out; > } > > + err = ufshcd_alloc_host(dev, &hba); > + if (err) { > + dev_err(&pdev->dev, "Allocation failed\n"); > + goto out; > + } > + > + hba->vops = get_variant_ops(&pdev->dev); > + > pm_runtime_set_active(&pdev->dev); > pm_runtime_enable(&pdev->dev); > > - err = ufshcd_init(dev, &hba, mmio_base, irq); > + err = ufshcd_init(hba, mmio_base, irq); > if (err) { > dev_err(dev, "Intialization failed\n"); > goto out_disable_rpm; > diff --git a/drivers/scsi/ufs/ufshcd.c b/drivers/scsi/ufs/ufshcd.c > index a0f5ac2..743696a 100644 > --- a/drivers/scsi/ufs/ufshcd.c > +++ b/drivers/scsi/ufs/ufshcd.c > @@ -174,13 +174,14 @@ static inline u32 ufshcd_get_ufs_version(struct ufs_hba *hba) > /** > * ufshcd_is_device_present - Check if any device connected to > * the host controller > - * @reg_hcs - host controller status register value > + * @hba: pointer to adapter instance > * > * Returns 1 if device present, 0 if no device detected > */ > -static inline int ufshcd_is_device_present(u32 reg_hcs) > +static inline int ufshcd_is_device_present(struct ufs_hba *hba) > { > - return (DEVICE_PRESENT & reg_hcs) ? 1 : 0; > + return (ufshcd_readl(hba, REG_CONTROLLER_STATUS) & > + DEVICE_PRESENT) ? 1 : 0; > } > > /** > @@ -1483,11 +1484,10 @@ out: > * @hba: per adapter instance > * > * To bring UFS host controller to operational state, > - * 1. Check if device is present > - * 2. Enable required interrupts > - * 3. Configure interrupt aggregation > - * 4. Program UTRL and UTMRL base addres > - * 5. Configure run-stop-registers > + * 1. Enable required interrupts > + * 2. Configure interrupt aggregation > + * 3. Program UTRL and UTMRL base addres > + * 4. Configure run-stop-registers > * > * Returns 0 on success, non-zero value on failure > */ > @@ -1496,14 +1496,6 @@ static int ufshcd_make_hba_operational(struct ufs_hba *hba) > int err = 0; > u32 reg; > > - /* check if device present */ > - reg = ufshcd_readl(hba, REG_CONTROLLER_STATUS); > - if (!ufshcd_is_device_present(reg)) { > - dev_err(hba->dev, "cc: Device not present\n"); > - err = -ENXIO; > - goto out; > - } > - > /* Enable required interrupts */ > ufshcd_enable_intr(hba, UFSHCD_ENABLE_INTRS); > > @@ -1524,6 +1516,7 @@ static int ufshcd_make_hba_operational(struct ufs_hba *hba) > * UCRDY, UTMRLDY and UTRLRDY bits must be 1 > * DEI, HEI bits must be 0 > */ > + reg = ufshcd_readl(hba, REG_CONTROLLER_STATUS); > if (!(ufshcd_get_lists_status(reg))) { > ufshcd_enable_run_stop_reg(hba); > } else { > @@ -1570,6 +1563,9 @@ static int ufshcd_hba_enable(struct ufs_hba *hba) > msleep(5); > } > > + if (hba->vops && hba->vops->hce_enable_notify) > + hba->vops->hce_enable_notify(hba, PRE_CHANGE); > + > /* start controller initialization sequence */ > ufshcd_hba_start(hba); > > @@ -1597,6 +1593,10 @@ static int ufshcd_hba_enable(struct ufs_hba *hba) > } > msleep(5); > } > + > + if (hba->vops && hba->vops->hce_enable_notify) > + hba->vops->hce_enable_notify(hba, POST_CHANGE); > + > return 0; > } > > @@ -1613,12 +1613,28 @@ static int ufshcd_link_startup(struct ufs_hba *hba) > /* enable UIC related interrupts */ > ufshcd_enable_intr(hba, UIC_COMMAND_COMPL); > > + if (hba->vops && hba->vops->link_startup_notify) > + hba->vops->link_startup_notify(hba, PRE_CHANGE); > + > ret = ufshcd_dme_link_startup(hba); > if (ret) > goto out; > > - ret = ufshcd_make_hba_operational(hba); > + /* check if device is detected by inter-connect layer */ > + if (!ufshcd_is_device_present(hba)) { > + dev_err(hba->dev, "%s: Device not present\n", __func__); > + ret = -ENXIO; > + goto out; > + } > > + /* Include any host controller configuration via UIC commands */ > + if (hba->vops && hba->vops->link_startup_notify) { > + ret = hba->vops->link_startup_notify(hba, POST_CHANGE); > + if (ret) > + goto out; > + } > + > + ret = ufshcd_make_hba_operational(hba); > out: > if (ret) > dev_err(hba->dev, "link startup failed %d\n", ret); > @@ -2810,6 +2826,61 @@ static struct scsi_host_template ufshcd_driver_template = { > .can_queue = UFSHCD_CAN_QUEUE, > }; > > +static int ufshcd_variant_hba_init(struct ufs_hba *hba) > +{ > + int err = 0; > + > + if (!hba->vops) > + goto out; > + > + if (hba->vops->init) { > + err = hba->vops->init(hba); > + if (err) > + goto out; > + } > + > + if (hba->vops->setup_clocks) { > + err = hba->vops->setup_clocks(hba, true); > + if (err) > + goto out_exit; > + } > + > + if (hba->vops->setup_regulators) { > + err = hba->vops->setup_regulators(hba, true); > + if (err) > + goto out_clks; > + } > + > + goto out; > + > +out_clks: > + if (hba->vops->setup_clocks) > + hba->vops->setup_clocks(hba, false); > +out_exit: > + if (hba->vops->exit) > + hba->vops->exit(hba); > +out: > + if (err) > + dev_err(hba->dev, "%s: variant %s init failed err %d\n", > + __func__, hba->vops ? hba->vops->name : "", err); > + return err; > +} > + > +static void ufshcd_variant_hba_exit(struct ufs_hba *hba) > +{ > + if (!hba->vops) > + return; > + > + if (hba->vops->setup_clocks) > + hba->vops->setup_clocks(hba, false); > + > + if (hba->vops->setup_regulators) > + hba->vops->setup_regulators(hba, false); > + > + if (hba->vops->exit) > + hba->vops->exit(hba); > +} > + > /** > * ufshcd_suspend - suspend power management function > * @hba: per adapter instance > @@ -2894,23 +2965,22 @@ void ufshcd_remove(struct ufs_hba *hba) > ufshcd_hba_stop(hba); > > scsi_host_put(hba->host); > + > + ufshcd_variant_hba_exit(hba); > } > EXPORT_SYMBOL_GPL(ufshcd_remove); > > /** > - * ufshcd_init - Driver initialization routine > + * ufshcd_alloc_host - allocate Host Bus Adapter (HBA) > * @dev: pointer to device handle > * @hba_handle: driver private handle > - * @mmio_base: base register address > - * @irq: Interrupt line of device > * Returns 0 on success, non-zero value on failure > */ > -int ufshcd_init(struct device *dev, struct ufs_hba **hba_handle, > - void __iomem *mmio_base, unsigned int irq) > +int ufshcd_alloc_host(struct device *dev, struct ufs_hba **hba_handle) > { > struct Scsi_Host *host; > struct ufs_hba *hba; > - int err; > + int err = 0; > > if (!dev) { > dev_err(dev, > @@ -2919,13 +2989,6 @@ int ufshcd_init(struct device *dev, struct ufs_hba **hba_handle, > goto out_error; > } > > - if (!mmio_base) { > - dev_err(dev, > - "Invalid memory reference for mmio_base is NULL\n"); > - err = -ENODEV; > - goto out_error; > - } > - > host = scsi_host_alloc(&ufshcd_driver_template, > sizeof(struct ufs_hba)); > if (!host) { > @@ -2936,9 +2999,40 @@ int ufshcd_init(struct device *dev, struct ufs_hba **hba_handle, > hba = shost_priv(host); > hba->host = host; > hba->dev = dev; > + *hba_handle = hba; > + > +out_error: > + return err; > +} > +EXPORT_SYMBOL(ufshcd_alloc_host); > + > +/** > + * ufshcd_init - Driver initialization routine > + * @hba: per-adapter instance > + * @mmio_base: base register address > + * @irq: Interrupt line of device > + * Returns 0 on success, non-zero value on failure > + */ > +int ufshcd_init(struct ufs_hba *hba, void __iomem *mmio_base, unsigned int irq) > +{ > + int err; > + struct Scsi_Host *host = hba->host; > + struct device *dev = hba->dev; > + > + if (!mmio_base) { > + dev_err(hba->dev, > + "Invalid memory reference for mmio_base is NULL\n"); > + err = -ENODEV; > + goto out_error; > + } > + > hba->mmio_base = mmio_base; > hba->irq = irq; > > + err = ufshcd_variant_hba_init(hba); > + if (err) > + goto out_error; > + > /* Read capabilities registers */ > ufshcd_hba_capabilities(hba); > > @@ -3010,8 +3104,6 @@ int ufshcd_init(struct device *dev, struct ufs_hba **hba_handle, > goto out_remove_scsi_host; > } > > - *hba_handle = hba; > - > /* Hold auto suspend until async scan completes */ > pm_runtime_get_sync(dev); > > @@ -3023,6 +3115,7 @@ out_remove_scsi_host: > scsi_remove_host(hba->host); > out_disable: > scsi_host_put(host); > + ufshcd_variant_hba_exit(hba); > out_error: > return err; > } > diff --git a/drivers/scsi/ufs/ufshcd.h b/drivers/scsi/ufs/ufshcd.h > index 8f5624e..72acbc7 100644 > --- a/drivers/scsi/ufs/ufshcd.h > +++ b/drivers/scsi/ufs/ufshcd.h > @@ -68,6 +68,8 @@ > #define UFSHCD "ufshcd" > #define UFSHCD_DRIVER_VERSION "0.2" > > +struct ufs_hba; > + > enum dev_cmd_type { > DEV_CMD_TYPE_NOP = 0x0, > DEV_CMD_TYPE_QUERY = 0x1, > @@ -152,6 +154,30 @@ struct ufs_dev_cmd { > struct ufs_query query; > }; > > +#define PRE_CHANGE 0 > +#define POST_CHANGE 1 > +/** > + * struct ufs_hba_variant_ops - variant specific callbacks > + * @name: variant name > + * @init: called when the driver is initialized > + * @exit: called to cleanup everything done in init > + * @setup_clocks: called before touching any of the controller registers > + * @setup_regulators: called before accessing the host controller > + * @hce_enable_notify: called before and after HCE enable bit is set to allow > + * variant specific Uni-Pro initialization. > + * @link_startup_notify: called before and after Link startup is carried out > + * to allow variant specific Uni-Pro initialization. > + */ > +struct ufs_hba_variant_ops { > + const char *name; > + int (*init)(struct ufs_hba *); > + void (*exit)(struct ufs_hba *); > + int (*setup_clocks)(struct ufs_hba *, bool); > + int (*setup_regulators)(struct ufs_hba *, bool); > + int (*hce_enable_notify)(struct ufs_hba *, bool); > + int (*link_startup_notify)(struct ufs_hba *, bool); > +}; > + > /** > * struct ufs_hba - per adapter private structure > * @mmio_base: UFSHCI base register address > @@ -171,6 +197,8 @@ struct ufs_dev_cmd { > * @nutrs: Transfer Request Queue depth supported by controller > * @nutmrs: Task Management Queue depth supported by controller > * @ufs_version: UFS Version to which controller complies > + * @vops: pointer to variant specific operations > + * @priv: pointer to variant specific private data > * @irq: Irq number of the controller > * @active_uic_cmd: handle of active UIC command > * @uic_cmd_mutex: mutex for uic command > @@ -217,6 +245,8 @@ struct ufs_hba { > int nutrs; > int nutmrs; > u32 ufs_version; > + struct ufs_hba_variant_ops *vops; > + void *priv; > unsigned int irq; > > struct uic_command *active_uic_cmd; > @@ -253,8 +283,8 @@ struct ufs_hba { > #define ufshcd_readl(hba, reg) \ > readl((hba)->mmio_base + (reg)) > > -int ufshcd_init(struct device *, struct ufs_hba ** , void __iomem * , > - unsigned int); > +int ufshcd_alloc_host(struct device *, struct ufs_hba **); > +int ufshcd_init(struct ufs_hba * , void __iomem * , unsigned int); > void ufshcd_remove(struct ufs_hba *); > > /** -- To unsubscribe from this list: send the line "unsubscribe linux-arm-msm" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
> > +static int ufshcd_variant_hba_init(struct ufs_hba *hba) > +{ > + int err = 0; > + > + if (!hba->vops) > + goto out; > + > + if (hba->vops->init) { > + err = hba->vops->init(hba); > + if (err) > + goto out; > + } > + > + if (hba->vops->setup_clocks) { > + err = hba->vops->setup_clocks(hba, true); > + if (err) > + goto out_exit; > + } > + > + if (hba->vops->setup_regulators) { > + err = hba->vops->setup_regulators(hba, true); > + if (err) > + goto out_clks; > + } > + > + goto out; > + > +out_clks: > + if (hba->vops->setup_clocks) > + hba->vops->setup_clocks(hba, false); > +out_exit: > + if (hba->vops->exit) > + hba->vops->exit(hba); > +out: > + if (err) > + dev_err(hba->dev, "%s: variant %s init failed err %d\n", > + __func__, hba->vops ? hba->vops->name : "", err); ^^^^^^^ a minor comment, 'hba->vops' will not be NULL here, > + return err; > +} > +
Hi Sujit, On Tue, August 27, 2013, Sujit Reddy Thumma wrote: > Some vendor specific controller versions might need to configure > vendor specific - registers, clocks, voltage regulators etc. to > initialize the host controller UTP layer and Uni-Pro stack. > Provide some common initialization operations that can be used > to configure vendor specifics. The methods can be extended in > future, for example, for power mode transitions. > > The operations are vendor/board specific and hence determined with > the help of compatible property in device tree. > > Signed-off-by: Sujit Reddy Thumma <sthumma@codeaurora.org> > --- > drivers/scsi/ufs/ufshcd-pci.c | 8 +- > drivers/scsi/ufs/ufshcd-pltfrm.c | 24 +++++- > drivers/scsi/ufs/ufshcd.c | 157 +++++++++++++++++++++++++++++++-------- > drivers/scsi/ufs/ufshcd.h | 34 ++++++++- > 4 files changed, 187 insertions(+), 36 deletions(-) > > diff --git a/drivers/scsi/ufs/ufshcd-pci.c b/drivers/scsi/ufs/ufshcd-pci.c > index a823cf4..6b0d299 100644 > --- a/drivers/scsi/ufs/ufshcd-pci.c > +++ b/drivers/scsi/ufs/ufshcd-pci.c > @@ -191,7 +191,13 @@ ufshcd_pci_probe(struct pci_dev *pdev, const struct pci_device_id *id) > return err; > } > > - err = ufshcd_init(&pdev->dev, &hba, mmio_base, pdev->irq); > + err = ufshcd_alloc_host(&pdev->dev, &hba); > + if (err) { > + dev_err(&pdev->dev, "Allocation failed\n"); > + return err; > + } > + > + err = ufshcd_init(hba, mmio_base, pdev->irq); > if (err) { > dev_err(&pdev->dev, "Initialization failed\n"); > return err; > diff --git a/drivers/scsi/ufs/ufshcd-pltfrm.c b/drivers/scsi/ufs/ufshcd-pltfrm.c > index 5e46232..9c94052 100644 > --- a/drivers/scsi/ufs/ufshcd-pltfrm.c > +++ b/drivers/scsi/ufs/ufshcd-pltfrm.c > @@ -35,9 +35,23 @@ > > #include <linux/platform_device.h> > #include <linux/pm_runtime.h> > +#include <linux/of.h> > > #include "ufshcd.h" > > +static const struct of_device_id ufs_of_match[]; > +static struct ufs_hba_variant_ops *get_variant_ops(struct device *dev) > +{ > + if (dev->of_node) { > + const struct of_device_id *match; > + match = of_match_node(ufs_of_match, dev->of_node); > + if (match) > + return (struct ufs_hba_variant_ops *)match->data; > + } > + > + return NULL; > +} > + > #ifdef CONFIG_PM > /** > * ufshcd_pltfrm_suspend - suspend power management function > @@ -150,10 +164,18 @@ static int ufshcd_pltfrm_probe(struct platform_device *pdev) > goto out; > } > > + err = ufshcd_alloc_host(dev, &hba); > + if (err) { > + dev_err(&pdev->dev, "Allocation failed\n"); > + goto out; > + } > + > + hba->vops = get_variant_ops(&pdev->dev); > + > pm_runtime_set_active(&pdev->dev); > pm_runtime_enable(&pdev->dev); > > - err = ufshcd_init(dev, &hba, mmio_base, irq); > + err = ufshcd_init(hba, mmio_base, irq); > if (err) { > dev_err(dev, "Intialization failed\n"); > goto out_disable_rpm; > diff --git a/drivers/scsi/ufs/ufshcd.c b/drivers/scsi/ufs/ufshcd.c > index a0f5ac2..743696a 100644 > --- a/drivers/scsi/ufs/ufshcd.c > +++ b/drivers/scsi/ufs/ufshcd.c > @@ -174,13 +174,14 @@ static inline u32 ufshcd_get_ufs_version(struct ufs_hba *hba) > /** > * ufshcd_is_device_present - Check if any device connected to > * the host controller > - * @reg_hcs - host controller status register value > + * @hba: pointer to adapter instance > * > * Returns 1 if device present, 0 if no device detected > */ > -static inline int ufshcd_is_device_present(u32 reg_hcs) > +static inline int ufshcd_is_device_present(struct ufs_hba *hba) > { > - return (DEVICE_PRESENT & reg_hcs) ? 1 : 0; > + return (ufshcd_readl(hba, REG_CONTROLLER_STATUS) & > + DEVICE_PRESENT) ? 1 : 0; > } > > /** > @@ -1483,11 +1484,10 @@ out: > * @hba: per adapter instance > * > * To bring UFS host controller to operational state, > - * 1. Check if device is present > - * 2. Enable required interrupts > - * 3. Configure interrupt aggregation > - * 4. Program UTRL and UTMRL base addres > - * 5. Configure run-stop-registers > + * 1. Enable required interrupts > + * 2. Configure interrupt aggregation > + * 3. Program UTRL and UTMRL base addres > + * 4. Configure run-stop-registers > * > * Returns 0 on success, non-zero value on failure > */ > @@ -1496,14 +1496,6 @@ static int ufshcd_make_hba_operational(struct ufs_hba *hba) > int err = 0; > u32 reg; > > - /* check if device present */ > - reg = ufshcd_readl(hba, REG_CONTROLLER_STATUS); > - if (!ufshcd_is_device_present(reg)) { > - dev_err(hba->dev, "cc: Device not present\n"); > - err = -ENXIO; > - goto out; > - } > - > /* Enable required interrupts */ > ufshcd_enable_intr(hba, UFSHCD_ENABLE_INTRS); > > @@ -1524,6 +1516,7 @@ static int ufshcd_make_hba_operational(struct ufs_hba *hba) > * UCRDY, UTMRLDY and UTRLRDY bits must be 1 > * DEI, HEI bits must be 0 > */ > + reg = ufshcd_readl(hba, REG_CONTROLLER_STATUS); > if (!(ufshcd_get_lists_status(reg))) { > ufshcd_enable_run_stop_reg(hba); > } else { > @@ -1570,6 +1563,9 @@ static int ufshcd_hba_enable(struct ufs_hba *hba) > msleep(5); > } > > + if (hba->vops && hba->vops->hce_enable_notify) > + hba->vops->hce_enable_notify(hba, PRE_CHANGE); > + > /* start controller initialization sequence */ > ufshcd_hba_start(hba); > > @@ -1597,6 +1593,10 @@ static int ufshcd_hba_enable(struct ufs_hba *hba) > } > msleep(5); > } > + > + if (hba->vops && hba->vops->hce_enable_notify) > + hba->vops->hce_enable_notify(hba, POST_CHANGE); > + > return 0; > } > > @@ -1613,12 +1613,28 @@ static int ufshcd_link_startup(struct ufs_hba *hba) > /* enable UIC related interrupts */ > ufshcd_enable_intr(hba, UIC_COMMAND_COMPL); > > + if (hba->vops && hba->vops->link_startup_notify) > + hba->vops->link_startup_notify(hba, PRE_CHANGE); > + > ret = ufshcd_dme_link_startup(hba); > if (ret) > goto out; > > - ret = ufshcd_make_hba_operational(hba); > + /* check if device is detected by inter-connect layer */ > + if (!ufshcd_is_device_present(hba)) { > + dev_err(hba->dev, "%s: Device not present\n", __func__); > + ret = -ENXIO; > + goto out; > + } > > + /* Include any host controller configuration via UIC commands */ > + if (hba->vops && hba->vops->link_startup_notify) { > + ret = hba->vops->link_startup_notify(hba, POST_CHANGE); > + if (ret) > + goto out; > + } > + > + ret = ufshcd_make_hba_operational(hba); > out: > if (ret) > dev_err(hba->dev, "link startup failed %d\n", ret); > @@ -2810,6 +2826,61 @@ static struct scsi_host_template ufshcd_driver_template = { > .can_queue = UFSHCD_CAN_QUEUE, > }; > > +static int ufshcd_variant_hba_init(struct ufs_hba *hba) > +{ > + int err = 0; > + > + if (!hba->vops) > + goto out; > + > + if (hba->vops->init) { > + err = hba->vops->init(hba); If init() is allowed to access host specific registers, setup_clocks() and setup_regulators() may precede init() generally. As seeing your description, it seems certain. <Quot> @init: called when the driver is initialized @setup_clocks: called before touching any of the controller registers @setup_regulators: called before accessing the host controller </Quot> Further, possibly init() implementation might substitute all? > + if (err) > + goto out; > + } > + > + if (hba->vops->setup_clocks) { > + err = hba->vops->setup_clocks(hba, true); > + if (err) > + goto out_exit; > + } > + > + if (hba->vops->setup_regulators) { > + err = hba->vops->setup_regulators(hba, true); > + if (err) > + goto out_clks; > + } > + > + goto out; > + > +out_clks: > + if (hba->vops->setup_clocks) > + hba->vops->setup_clocks(hba, false); > +out_exit: > + if (hba->vops->exit) > + hba->vops->exit(hba); > +out: > + if (err) > + dev_err(hba->dev, "%s: variant %s init failed err %d\n", > + __func__, hba->vops ? hba->vops->name : "", err); > + return err; > +} > + > +static void ufshcd_variant_hba_exit(struct ufs_hba *hba) > +{ > + if (!hba->vops) > + return; > + > + if (hba->vops->setup_clocks) > + hba->vops->setup_clocks(hba, false); > + > + if (hba->vops->setup_regulators) > + hba->vops->setup_regulators(hba, false); > + > + if (hba->vops->exit) > + hba->vops->exit(hba); It's similar with init() case above. > +} > + > /** > * ufshcd_suspend - suspend power management function > * @hba: per adapter instance > @@ -2894,23 +2965,22 @@ void ufshcd_remove(struct ufs_hba *hba) > ufshcd_hba_stop(hba); > > scsi_host_put(hba->host); > + > + ufshcd_variant_hba_exit(hba); > } > EXPORT_SYMBOL_GPL(ufshcd_remove); > > /** > - * ufshcd_init - Driver initialization routine > + * ufshcd_alloc_host - allocate Host Bus Adapter (HBA) > * @dev: pointer to device handle > * @hba_handle: driver private handle > - * @mmio_base: base register address > - * @irq: Interrupt line of device > * Returns 0 on success, non-zero value on failure > */ > -int ufshcd_init(struct device *dev, struct ufs_hba **hba_handle, > - void __iomem *mmio_base, unsigned int irq) > +int ufshcd_alloc_host(struct device *dev, struct ufs_hba **hba_handle) > { > struct Scsi_Host *host; > struct ufs_hba *hba; > - int err; > + int err = 0; > > if (!dev) { > dev_err(dev, > @@ -2919,13 +2989,6 @@ int ufshcd_init(struct device *dev, struct ufs_hba **hba_handle, > goto out_error; > } > > - if (!mmio_base) { > - dev_err(dev, > - "Invalid memory reference for mmio_base is NULL\n"); > - err = -ENODEV; > - goto out_error; > - } > - > host = scsi_host_alloc(&ufshcd_driver_template, > sizeof(struct ufs_hba)); > if (!host) { > @@ -2936,9 +2999,40 @@ int ufshcd_init(struct device *dev, struct ufs_hba **hba_handle, > hba = shost_priv(host); > hba->host = host; > hba->dev = dev; > + *hba_handle = hba; > + > +out_error: > + return err; > +} > +EXPORT_SYMBOL(ufshcd_alloc_host); > + > +/** > + * ufshcd_init - Driver initialization routine > + * @hba: per-adapter instance > + * @mmio_base: base register address > + * @irq: Interrupt line of device > + * Returns 0 on success, non-zero value on failure > + */ > +int ufshcd_init(struct ufs_hba *hba, void __iomem *mmio_base, unsigned int irq) > +{ > + int err; > + struct Scsi_Host *host = hba->host; > + struct device *dev = hba->dev; > + > + if (!mmio_base) { > + dev_err(hba->dev, > + "Invalid memory reference for mmio_base is NULL\n"); > + err = -ENODEV; > + goto out_error; > + } > + > hba->mmio_base = mmio_base; > hba->irq = irq; > > + err = ufshcd_variant_hba_init(hba); > + if (err) > + goto out_error; > + > /* Read capabilities registers */ > ufshcd_hba_capabilities(hba); > > @@ -3010,8 +3104,6 @@ int ufshcd_init(struct device *dev, struct ufs_hba **hba_handle, > goto out_remove_scsi_host; > } > > - *hba_handle = hba; > - > /* Hold auto suspend until async scan completes */ > pm_runtime_get_sync(dev); > > @@ -3023,6 +3115,7 @@ out_remove_scsi_host: > scsi_remove_host(hba->host); > out_disable: > scsi_host_put(host); > + ufshcd_variant_hba_exit(hba); > out_error: > return err; > } > diff --git a/drivers/scsi/ufs/ufshcd.h b/drivers/scsi/ufs/ufshcd.h > index 8f5624e..72acbc7 100644 > --- a/drivers/scsi/ufs/ufshcd.h > +++ b/drivers/scsi/ufs/ufshcd.h > @@ -68,6 +68,8 @@ > #define UFSHCD "ufshcd" > #define UFSHCD_DRIVER_VERSION "0.2" > > +struct ufs_hba; > + > enum dev_cmd_type { > DEV_CMD_TYPE_NOP = 0x0, > DEV_CMD_TYPE_QUERY = 0x1, > @@ -152,6 +154,30 @@ struct ufs_dev_cmd { > struct ufs_query query; > }; > > +#define PRE_CHANGE 0 > +#define POST_CHANGE 1 > +/** > + * struct ufs_hba_variant_ops - variant specific callbacks > + * @name: variant name > + * @init: called when the driver is initialized > + * @exit: called to cleanup everything done in init > + * @setup_clocks: called before touching any of the controller registers > + * @setup_regulators: called before accessing the host controller > + * @hce_enable_notify: called before and after HCE enable bit is set to allow > + * variant specific Uni-Pro initialization. > + * @link_startup_notify: called before and after Link startup is carried out > + * to allow variant specific Uni-Pro initialization. > + */ > +struct ufs_hba_variant_ops { > + const char *name; > + int (*init)(struct ufs_hba *); > + void (*exit)(struct ufs_hba *); > + int (*setup_clocks)(struct ufs_hba *, bool); > + int (*setup_regulators)(struct ufs_hba *, bool); > + int (*hce_enable_notify)(struct ufs_hba *, bool); I agree on specific link_startup as HCI specification mentions. Actually we also have implemented similar thing in our host driver. But I'm not sure of a necessity of hce_enable_notify. Could you explain for that? > + int (*link_startup_notify)(struct ufs_hba *, bool); If a host specific implementation doesn't have two fully, one of them will be called with flag unnecessarily. How about separating into two functions[pre_, post_] for link_startup? > +}; > + > /** > * struct ufs_hba - per adapter private structure > * @mmio_base: UFSHCI base register address > @@ -171,6 +197,8 @@ struct ufs_dev_cmd { > * @nutrs: Transfer Request Queue depth supported by controller > * @nutmrs: Task Management Queue depth supported by controller > * @ufs_version: UFS Version to which controller complies > + * @vops: pointer to variant specific operations > + * @priv: pointer to variant specific private data > * @irq: Irq number of the controller > * @active_uic_cmd: handle of active UIC command > * @uic_cmd_mutex: mutex for uic command > @@ -217,6 +245,8 @@ struct ufs_hba { > int nutrs; > int nutmrs; > u32 ufs_version; > + struct ufs_hba_variant_ops *vops; 'const' declaration is expected. Thanks, Seungwon Jeon > + void *priv; > unsigned int irq; > > struct uic_command *active_uic_cmd; > @@ -253,8 +283,8 @@ struct ufs_hba { > #define ufshcd_readl(hba, reg) \ > readl((hba)->mmio_base + (reg)) > > -int ufshcd_init(struct device *, struct ufs_hba ** , void __iomem * , > - unsigned int); > +int ufshcd_alloc_host(struct device *, struct ufs_hba **); > +int ufshcd_init(struct ufs_hba * , void __iomem * , unsigned int); > void ufshcd_remove(struct ufs_hba *); > > /** > -- > QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member > of Code Aurora Forum, hosted by The Linux Foundation. > > -- > To unsubscribe from this list: send the line "unsubscribe linux-scsi" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html -- To unsubscribe from this list: send the line "unsubscribe linux-arm-msm" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 9/9/2013 5:03 PM, Seungwon Jeon wrote: > Hi Sujit, > > On Tue, August 27, 2013, Sujit Reddy Thumma wrote: >> Some vendor specific controller versions might need to configure >> vendor specific - registers, clocks, voltage regulators etc. to >> initialize the host controller UTP layer and Uni-Pro stack. >> Provide some common initialization operations that can be used >> to configure vendor specifics. The methods can be extended in >> future, for example, for power mode transitions. >> >> The operations are vendor/board specific and hence determined with >> the help of compatible property in device tree. >> >> Signed-off-by: Sujit Reddy Thumma <sthumma@codeaurora.org> >> --- >> drivers/scsi/ufs/ufshcd-pci.c | 8 +- >> drivers/scsi/ufs/ufshcd-pltfrm.c | 24 +++++- >> drivers/scsi/ufs/ufshcd.c | 157 +++++++++++++++++++++++++++++++-------- >> drivers/scsi/ufs/ufshcd.h | 34 ++++++++- >> 4 files changed, 187 insertions(+), 36 deletions(-) >> >> diff --git a/drivers/scsi/ufs/ufshcd-pci.c b/drivers/scsi/ufs/ufshcd-pci.c >> index a823cf4..6b0d299 100644 >> --- a/drivers/scsi/ufs/ufshcd-pci.c >> +++ b/drivers/scsi/ufs/ufshcd-pci.c >> @@ -191,7 +191,13 @@ ufshcd_pci_probe(struct pci_dev *pdev, const struct pci_device_id *id) >> return err; >> } >> >> - err = ufshcd_init(&pdev->dev, &hba, mmio_base, pdev->irq); >> + err = ufshcd_alloc_host(&pdev->dev, &hba); >> + if (err) { >> + dev_err(&pdev->dev, "Allocation failed\n"); >> + return err; >> + } >> + >> + err = ufshcd_init(hba, mmio_base, pdev->irq); >> if (err) { >> dev_err(&pdev->dev, "Initialization failed\n"); >> return err; >> diff --git a/drivers/scsi/ufs/ufshcd-pltfrm.c b/drivers/scsi/ufs/ufshcd-pltfrm.c >> index 5e46232..9c94052 100644 >> --- a/drivers/scsi/ufs/ufshcd-pltfrm.c >> +++ b/drivers/scsi/ufs/ufshcd-pltfrm.c >> @@ -35,9 +35,23 @@ >> >> #include <linux/platform_device.h> >> #include <linux/pm_runtime.h> >> +#include <linux/of.h> >> >> #include "ufshcd.h" >> >> +static const struct of_device_id ufs_of_match[]; >> +static struct ufs_hba_variant_ops *get_variant_ops(struct device *dev) >> +{ >> + if (dev->of_node) { >> + const struct of_device_id *match; >> + match = of_match_node(ufs_of_match, dev->of_node); >> + if (match) >> + return (struct ufs_hba_variant_ops *)match->data; >> + } >> + >> + return NULL; >> +} >> + >> #ifdef CONFIG_PM >> /** >> * ufshcd_pltfrm_suspend - suspend power management function >> @@ -150,10 +164,18 @@ static int ufshcd_pltfrm_probe(struct platform_device *pdev) >> goto out; >> } >> >> + err = ufshcd_alloc_host(dev, &hba); >> + if (err) { >> + dev_err(&pdev->dev, "Allocation failed\n"); >> + goto out; >> + } >> + >> + hba->vops = get_variant_ops(&pdev->dev); >> + >> pm_runtime_set_active(&pdev->dev); >> pm_runtime_enable(&pdev->dev); >> >> - err = ufshcd_init(dev, &hba, mmio_base, irq); >> + err = ufshcd_init(hba, mmio_base, irq); >> if (err) { >> dev_err(dev, "Intialization failed\n"); >> goto out_disable_rpm; >> diff --git a/drivers/scsi/ufs/ufshcd.c b/drivers/scsi/ufs/ufshcd.c >> index a0f5ac2..743696a 100644 >> --- a/drivers/scsi/ufs/ufshcd.c >> +++ b/drivers/scsi/ufs/ufshcd.c >> @@ -174,13 +174,14 @@ static inline u32 ufshcd_get_ufs_version(struct ufs_hba *hba) >> /** >> * ufshcd_is_device_present - Check if any device connected to >> * the host controller >> - * @reg_hcs - host controller status register value >> + * @hba: pointer to adapter instance >> * >> * Returns 1 if device present, 0 if no device detected >> */ >> -static inline int ufshcd_is_device_present(u32 reg_hcs) >> +static inline int ufshcd_is_device_present(struct ufs_hba *hba) >> { >> - return (DEVICE_PRESENT & reg_hcs) ? 1 : 0; >> + return (ufshcd_readl(hba, REG_CONTROLLER_STATUS) & >> + DEVICE_PRESENT) ? 1 : 0; >> } >> >> /** >> @@ -1483,11 +1484,10 @@ out: >> * @hba: per adapter instance >> * >> * To bring UFS host controller to operational state, >> - * 1. Check if device is present >> - * 2. Enable required interrupts >> - * 3. Configure interrupt aggregation >> - * 4. Program UTRL and UTMRL base addres >> - * 5. Configure run-stop-registers >> + * 1. Enable required interrupts >> + * 2. Configure interrupt aggregation >> + * 3. Program UTRL and UTMRL base addres >> + * 4. Configure run-stop-registers >> * >> * Returns 0 on success, non-zero value on failure >> */ >> @@ -1496,14 +1496,6 @@ static int ufshcd_make_hba_operational(struct ufs_hba *hba) >> int err = 0; >> u32 reg; >> >> - /* check if device present */ >> - reg = ufshcd_readl(hba, REG_CONTROLLER_STATUS); >> - if (!ufshcd_is_device_present(reg)) { >> - dev_err(hba->dev, "cc: Device not present\n"); >> - err = -ENXIO; >> - goto out; >> - } >> - >> /* Enable required interrupts */ >> ufshcd_enable_intr(hba, UFSHCD_ENABLE_INTRS); >> >> @@ -1524,6 +1516,7 @@ static int ufshcd_make_hba_operational(struct ufs_hba *hba) >> * UCRDY, UTMRLDY and UTRLRDY bits must be 1 >> * DEI, HEI bits must be 0 >> */ >> + reg = ufshcd_readl(hba, REG_CONTROLLER_STATUS); >> if (!(ufshcd_get_lists_status(reg))) { >> ufshcd_enable_run_stop_reg(hba); >> } else { >> @@ -1570,6 +1563,9 @@ static int ufshcd_hba_enable(struct ufs_hba *hba) >> msleep(5); >> } >> >> + if (hba->vops && hba->vops->hce_enable_notify) >> + hba->vops->hce_enable_notify(hba, PRE_CHANGE); >> + >> /* start controller initialization sequence */ >> ufshcd_hba_start(hba); >> >> @@ -1597,6 +1593,10 @@ static int ufshcd_hba_enable(struct ufs_hba *hba) >> } >> msleep(5); >> } >> + >> + if (hba->vops && hba->vops->hce_enable_notify) >> + hba->vops->hce_enable_notify(hba, POST_CHANGE); >> + >> return 0; >> } >> >> @@ -1613,12 +1613,28 @@ static int ufshcd_link_startup(struct ufs_hba *hba) >> /* enable UIC related interrupts */ >> ufshcd_enable_intr(hba, UIC_COMMAND_COMPL); >> >> + if (hba->vops && hba->vops->link_startup_notify) >> + hba->vops->link_startup_notify(hba, PRE_CHANGE); >> + >> ret = ufshcd_dme_link_startup(hba); >> if (ret) >> goto out; >> >> - ret = ufshcd_make_hba_operational(hba); >> + /* check if device is detected by inter-connect layer */ >> + if (!ufshcd_is_device_present(hba)) { >> + dev_err(hba->dev, "%s: Device not present\n", __func__); >> + ret = -ENXIO; >> + goto out; >> + } >> >> + /* Include any host controller configuration via UIC commands */ >> + if (hba->vops && hba->vops->link_startup_notify) { >> + ret = hba->vops->link_startup_notify(hba, POST_CHANGE); >> + if (ret) >> + goto out; >> + } >> + >> + ret = ufshcd_make_hba_operational(hba); >> out: >> if (ret) >> dev_err(hba->dev, "link startup failed %d\n", ret); >> @@ -2810,6 +2826,61 @@ static struct scsi_host_template ufshcd_driver_template = { >> .can_queue = UFSHCD_CAN_QUEUE, >> }; >> >> +static int ufshcd_variant_hba_init(struct ufs_hba *hba) >> +{ >> + int err = 0; >> + >> + if (!hba->vops) >> + goto out; >> + >> + if (hba->vops->init) { >> + err = hba->vops->init(hba); > If init() is allowed to access host specific registers, > setup_clocks() and setup_regulators() may precede init() generally. > As seeing your description, it seems certain. > <Quot> > @init: called when the driver is initialized > @setup_clocks: called before touching any of the controller registers > @setup_regulators: called before accessing the host controller > </Quot> > > Further, possibly init() implementation might substitute all? Yes, we can. The reason I would like to keep it different because it can be used during power management. For example, - init -> clk_get(); setup_clocks -> clk_prepare_enable(); ufshcd_runtime_suspend() { ... hba->vops->setup_clocks(false); } ufshcd_runtime_resume() { hba->vops->setup_clocks(true); ... } > >> + if (err) >> + goto out; >> + } >> + >> + if (hba->vops->setup_clocks) { >> + err = hba->vops->setup_clocks(hba, true); >> + if (err) >> + goto out_exit; >> + } >> + >> + if (hba->vops->setup_regulators) { >> + err = hba->vops->setup_regulators(hba, true); >> + if (err) >> + goto out_clks; >> + } >> + >> + goto out; >> + >> +out_clks: >> + if (hba->vops->setup_clocks) >> + hba->vops->setup_clocks(hba, false); >> +out_exit: >> + if (hba->vops->exit) >> + hba->vops->exit(hba); >> +out: >> + if (err) >> + dev_err(hba->dev, "%s: variant %s init failed err %d\n", >> + __func__, hba->vops ? hba->vops->name : "", err); >> + return err; >> +} >> + >> +static void ufshcd_variant_hba_exit(struct ufs_hba *hba) >> +{ >> + if (!hba->vops) >> + return; >> + >> + if (hba->vops->setup_clocks) >> + hba->vops->setup_clocks(hba, false); >> + >> + if (hba->vops->setup_regulators) >> + hba->vops->setup_regulators(hba, false); >> + >> + if (hba->vops->exit) >> + hba->vops->exit(hba); > It's similar with init() case above. Just to keep consistent with init() procedures. > >> +} >> + >> /** >> * ufshcd_suspend - suspend power management function >> * @hba: per adapter instance >> @@ -2894,23 +2965,22 @@ void ufshcd_remove(struct ufs_hba *hba) >> ufshcd_hba_stop(hba); >> >> scsi_host_put(hba->host); >> + >> + ufshcd_variant_hba_exit(hba); >> } >> EXPORT_SYMBOL_GPL(ufshcd_remove); >> >> /** >> - * ufshcd_init - Driver initialization routine >> + * ufshcd_alloc_host - allocate Host Bus Adapter (HBA) >> * @dev: pointer to device handle >> * @hba_handle: driver private handle >> - * @mmio_base: base register address >> - * @irq: Interrupt line of device >> * Returns 0 on success, non-zero value on failure >> */ >> -int ufshcd_init(struct device *dev, struct ufs_hba **hba_handle, >> - void __iomem *mmio_base, unsigned int irq) >> +int ufshcd_alloc_host(struct device *dev, struct ufs_hba **hba_handle) >> { >> struct Scsi_Host *host; >> struct ufs_hba *hba; >> - int err; >> + int err = 0; >> >> if (!dev) { >> dev_err(dev, >> @@ -2919,13 +2989,6 @@ int ufshcd_init(struct device *dev, struct ufs_hba **hba_handle, >> goto out_error; >> } >> >> - if (!mmio_base) { >> - dev_err(dev, >> - "Invalid memory reference for mmio_base is NULL\n"); >> - err = -ENODEV; >> - goto out_error; >> - } >> - >> host = scsi_host_alloc(&ufshcd_driver_template, >> sizeof(struct ufs_hba)); >> if (!host) { >> @@ -2936,9 +2999,40 @@ int ufshcd_init(struct device *dev, struct ufs_hba **hba_handle, >> hba = shost_priv(host); >> hba->host = host; >> hba->dev = dev; >> + *hba_handle = hba; >> + >> +out_error: >> + return err; >> +} >> +EXPORT_SYMBOL(ufshcd_alloc_host); >> + >> +/** >> + * ufshcd_init - Driver initialization routine >> + * @hba: per-adapter instance >> + * @mmio_base: base register address >> + * @irq: Interrupt line of device >> + * Returns 0 on success, non-zero value on failure >> + */ >> +int ufshcd_init(struct ufs_hba *hba, void __iomem *mmio_base, unsigned int irq) >> +{ >> + int err; >> + struct Scsi_Host *host = hba->host; >> + struct device *dev = hba->dev; >> + >> + if (!mmio_base) { >> + dev_err(hba->dev, >> + "Invalid memory reference for mmio_base is NULL\n"); >> + err = -ENODEV; >> + goto out_error; >> + } >> + >> hba->mmio_base = mmio_base; >> hba->irq = irq; >> >> + err = ufshcd_variant_hba_init(hba); >> + if (err) >> + goto out_error; >> + >> /* Read capabilities registers */ >> ufshcd_hba_capabilities(hba); >> >> @@ -3010,8 +3104,6 @@ int ufshcd_init(struct device *dev, struct ufs_hba **hba_handle, >> goto out_remove_scsi_host; >> } >> >> - *hba_handle = hba; >> - >> /* Hold auto suspend until async scan completes */ >> pm_runtime_get_sync(dev); >> >> @@ -3023,6 +3115,7 @@ out_remove_scsi_host: >> scsi_remove_host(hba->host); >> out_disable: >> scsi_host_put(host); >> + ufshcd_variant_hba_exit(hba); >> out_error: >> return err; >> } >> diff --git a/drivers/scsi/ufs/ufshcd.h b/drivers/scsi/ufs/ufshcd.h >> index 8f5624e..72acbc7 100644 >> --- a/drivers/scsi/ufs/ufshcd.h >> +++ b/drivers/scsi/ufs/ufshcd.h >> @@ -68,6 +68,8 @@ >> #define UFSHCD "ufshcd" >> #define UFSHCD_DRIVER_VERSION "0.2" >> >> +struct ufs_hba; >> + >> enum dev_cmd_type { >> DEV_CMD_TYPE_NOP = 0x0, >> DEV_CMD_TYPE_QUERY = 0x1, >> @@ -152,6 +154,30 @@ struct ufs_dev_cmd { >> struct ufs_query query; >> }; >> >> +#define PRE_CHANGE 0 >> +#define POST_CHANGE 1 >> +/** >> + * struct ufs_hba_variant_ops - variant specific callbacks >> + * @name: variant name >> + * @init: called when the driver is initialized >> + * @exit: called to cleanup everything done in init >> + * @setup_clocks: called before touching any of the controller registers >> + * @setup_regulators: called before accessing the host controller >> + * @hce_enable_notify: called before and after HCE enable bit is set to allow >> + * variant specific Uni-Pro initialization. >> + * @link_startup_notify: called before and after Link startup is carried out >> + * to allow variant specific Uni-Pro initialization. >> + */ >> +struct ufs_hba_variant_ops { >> + const char *name; >> + int (*init)(struct ufs_hba *); >> + void (*exit)(struct ufs_hba *); >> + int (*setup_clocks)(struct ufs_hba *, bool); >> + int (*setup_regulators)(struct ufs_hba *, bool); >> + int (*hce_enable_notify)(struct ufs_hba *, bool); > I agree on specific link_startup as HCI specification mentions. > Actually we also have implemented similar thing in our host driver. > But I'm not sure of a necessity of hce_enable_notify. Could you explain for that? I am not sure about other M-PHY implementations but in our h/w there are certain M-PHY specific configurations that needs to be done before enabling the host controller and also some state machine check after resetting the controller. > >> + int (*link_startup_notify)(struct ufs_hba *, bool); > If a host specific implementation doesn't have two fully, > one of them will be called with flag unnecessarily. > How about separating into two functions[pre_, post_] for link_startup? Agree. > >> +}; >> + >> /** >> * struct ufs_hba - per adapter private structure >> * @mmio_base: UFSHCI base register address >> @@ -171,6 +197,8 @@ struct ufs_dev_cmd { >> * @nutrs: Transfer Request Queue depth supported by controller >> * @nutmrs: Task Management Queue depth supported by controller >> * @ufs_version: UFS Version to which controller complies >> + * @vops: pointer to variant specific operations >> + * @priv: pointer to variant specific private data >> * @irq: Irq number of the controller >> * @active_uic_cmd: handle of active UIC command >> * @uic_cmd_mutex: mutex for uic command >> @@ -217,6 +245,8 @@ struct ufs_hba { >> int nutrs; >> int nutmrs; >> u32 ufs_version; >> + struct ufs_hba_variant_ops *vops; > 'const' declaration is expected. Agree. > > Thanks, > Seungwon Jeon > >> + void *priv; >> unsigned int irq; >> >> struct uic_command *active_uic_cmd; >> @@ -253,8 +283,8 @@ struct ufs_hba { >> #define ufshcd_readl(hba, reg) \ >> readl((hba)->mmio_base + (reg)) >> >> -int ufshcd_init(struct device *, struct ufs_hba ** , void __iomem * , >> - unsigned int); >> +int ufshcd_alloc_host(struct device *, struct ufs_hba **); >> +int ufshcd_init(struct ufs_hba * , void __iomem * , unsigned int); >> void ufshcd_remove(struct ufs_hba *); >> >> /** >> -- >> QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member >> of Code Aurora Forum, hosted by The Linux Foundation. >> >> -- >> To unsubscribe from this list: send the line "unsubscribe linux-scsi" 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/scsi/ufs/ufshcd-pci.c b/drivers/scsi/ufs/ufshcd-pci.c index a823cf4..6b0d299 100644 --- a/drivers/scsi/ufs/ufshcd-pci.c +++ b/drivers/scsi/ufs/ufshcd-pci.c @@ -191,7 +191,13 @@ ufshcd_pci_probe(struct pci_dev *pdev, const struct pci_device_id *id) return err; } - err = ufshcd_init(&pdev->dev, &hba, mmio_base, pdev->irq); + err = ufshcd_alloc_host(&pdev->dev, &hba); + if (err) { + dev_err(&pdev->dev, "Allocation failed\n"); + return err; + } + + err = ufshcd_init(hba, mmio_base, pdev->irq); if (err) { dev_err(&pdev->dev, "Initialization failed\n"); return err; diff --git a/drivers/scsi/ufs/ufshcd-pltfrm.c b/drivers/scsi/ufs/ufshcd-pltfrm.c index 5e46232..9c94052 100644 --- a/drivers/scsi/ufs/ufshcd-pltfrm.c +++ b/drivers/scsi/ufs/ufshcd-pltfrm.c @@ -35,9 +35,23 @@ #include <linux/platform_device.h> #include <linux/pm_runtime.h> +#include <linux/of.h> #include "ufshcd.h" +static const struct of_device_id ufs_of_match[]; +static struct ufs_hba_variant_ops *get_variant_ops(struct device *dev) +{ + if (dev->of_node) { + const struct of_device_id *match; + match = of_match_node(ufs_of_match, dev->of_node); + if (match) + return (struct ufs_hba_variant_ops *)match->data; + } + + return NULL; +} + #ifdef CONFIG_PM /** * ufshcd_pltfrm_suspend - suspend power management function @@ -150,10 +164,18 @@ static int ufshcd_pltfrm_probe(struct platform_device *pdev) goto out; } + err = ufshcd_alloc_host(dev, &hba); + if (err) { + dev_err(&pdev->dev, "Allocation failed\n"); + goto out; + } + + hba->vops = get_variant_ops(&pdev->dev); + pm_runtime_set_active(&pdev->dev); pm_runtime_enable(&pdev->dev); - err = ufshcd_init(dev, &hba, mmio_base, irq); + err = ufshcd_init(hba, mmio_base, irq); if (err) { dev_err(dev, "Intialization failed\n"); goto out_disable_rpm; diff --git a/drivers/scsi/ufs/ufshcd.c b/drivers/scsi/ufs/ufshcd.c index a0f5ac2..743696a 100644 --- a/drivers/scsi/ufs/ufshcd.c +++ b/drivers/scsi/ufs/ufshcd.c @@ -174,13 +174,14 @@ static inline u32 ufshcd_get_ufs_version(struct ufs_hba *hba) /** * ufshcd_is_device_present - Check if any device connected to * the host controller - * @reg_hcs - host controller status register value + * @hba: pointer to adapter instance * * Returns 1 if device present, 0 if no device detected */ -static inline int ufshcd_is_device_present(u32 reg_hcs) +static inline int ufshcd_is_device_present(struct ufs_hba *hba) { - return (DEVICE_PRESENT & reg_hcs) ? 1 : 0; + return (ufshcd_readl(hba, REG_CONTROLLER_STATUS) & + DEVICE_PRESENT) ? 1 : 0; } /** @@ -1483,11 +1484,10 @@ out: * @hba: per adapter instance * * To bring UFS host controller to operational state, - * 1. Check if device is present - * 2. Enable required interrupts - * 3. Configure interrupt aggregation - * 4. Program UTRL and UTMRL base addres - * 5. Configure run-stop-registers + * 1. Enable required interrupts + * 2. Configure interrupt aggregation + * 3. Program UTRL and UTMRL base addres + * 4. Configure run-stop-registers * * Returns 0 on success, non-zero value on failure */ @@ -1496,14 +1496,6 @@ static int ufshcd_make_hba_operational(struct ufs_hba *hba) int err = 0; u32 reg; - /* check if device present */ - reg = ufshcd_readl(hba, REG_CONTROLLER_STATUS); - if (!ufshcd_is_device_present(reg)) { - dev_err(hba->dev, "cc: Device not present\n"); - err = -ENXIO; - goto out; - } - /* Enable required interrupts */ ufshcd_enable_intr(hba, UFSHCD_ENABLE_INTRS); @@ -1524,6 +1516,7 @@ static int ufshcd_make_hba_operational(struct ufs_hba *hba) * UCRDY, UTMRLDY and UTRLRDY bits must be 1 * DEI, HEI bits must be 0 */ + reg = ufshcd_readl(hba, REG_CONTROLLER_STATUS); if (!(ufshcd_get_lists_status(reg))) { ufshcd_enable_run_stop_reg(hba); } else { @@ -1570,6 +1563,9 @@ static int ufshcd_hba_enable(struct ufs_hba *hba) msleep(5); } + if (hba->vops && hba->vops->hce_enable_notify) + hba->vops->hce_enable_notify(hba, PRE_CHANGE); + /* start controller initialization sequence */ ufshcd_hba_start(hba); @@ -1597,6 +1593,10 @@ static int ufshcd_hba_enable(struct ufs_hba *hba) } msleep(5); } + + if (hba->vops && hba->vops->hce_enable_notify) + hba->vops->hce_enable_notify(hba, POST_CHANGE); + return 0; } @@ -1613,12 +1613,28 @@ static int ufshcd_link_startup(struct ufs_hba *hba) /* enable UIC related interrupts */ ufshcd_enable_intr(hba, UIC_COMMAND_COMPL); + if (hba->vops && hba->vops->link_startup_notify) + hba->vops->link_startup_notify(hba, PRE_CHANGE); + ret = ufshcd_dme_link_startup(hba); if (ret) goto out; - ret = ufshcd_make_hba_operational(hba); + /* check if device is detected by inter-connect layer */ + if (!ufshcd_is_device_present(hba)) { + dev_err(hba->dev, "%s: Device not present\n", __func__); + ret = -ENXIO; + goto out; + } + /* Include any host controller configuration via UIC commands */ + if (hba->vops && hba->vops->link_startup_notify) { + ret = hba->vops->link_startup_notify(hba, POST_CHANGE); + if (ret) + goto out; + } + + ret = ufshcd_make_hba_operational(hba); out: if (ret) dev_err(hba->dev, "link startup failed %d\n", ret); @@ -2810,6 +2826,61 @@ static struct scsi_host_template ufshcd_driver_template = { .can_queue = UFSHCD_CAN_QUEUE, }; +static int ufshcd_variant_hba_init(struct ufs_hba *hba) +{ + int err = 0; + + if (!hba->vops) + goto out; + + if (hba->vops->init) { + err = hba->vops->init(hba); + if (err) + goto out; + } + + if (hba->vops->setup_clocks) { + err = hba->vops->setup_clocks(hba, true); + if (err) + goto out_exit; + } + + if (hba->vops->setup_regulators) { + err = hba->vops->setup_regulators(hba, true); + if (err) + goto out_clks; + } + + goto out; + +out_clks: + if (hba->vops->setup_clocks) + hba->vops->setup_clocks(hba, false); +out_exit: + if (hba->vops->exit) + hba->vops->exit(hba); +out: + if (err) + dev_err(hba->dev, "%s: variant %s init failed err %d\n", + __func__, hba->vops ? hba->vops->name : "", err); + return err; +} + +static void ufshcd_variant_hba_exit(struct ufs_hba *hba) +{ + if (!hba->vops) + return; + + if (hba->vops->setup_clocks) + hba->vops->setup_clocks(hba, false); + + if (hba->vops->setup_regulators) + hba->vops->setup_regulators(hba, false); + + if (hba->vops->exit) + hba->vops->exit(hba); +} + /** * ufshcd_suspend - suspend power management function * @hba: per adapter instance @@ -2894,23 +2965,22 @@ void ufshcd_remove(struct ufs_hba *hba) ufshcd_hba_stop(hba); scsi_host_put(hba->host); + + ufshcd_variant_hba_exit(hba); } EXPORT_SYMBOL_GPL(ufshcd_remove); /** - * ufshcd_init - Driver initialization routine + * ufshcd_alloc_host - allocate Host Bus Adapter (HBA) * @dev: pointer to device handle * @hba_handle: driver private handle - * @mmio_base: base register address - * @irq: Interrupt line of device * Returns 0 on success, non-zero value on failure */ -int ufshcd_init(struct device *dev, struct ufs_hba **hba_handle, - void __iomem *mmio_base, unsigned int irq) +int ufshcd_alloc_host(struct device *dev, struct ufs_hba **hba_handle) { struct Scsi_Host *host; struct ufs_hba *hba; - int err; + int err = 0; if (!dev) { dev_err(dev, @@ -2919,13 +2989,6 @@ int ufshcd_init(struct device *dev, struct ufs_hba **hba_handle, goto out_error; } - if (!mmio_base) { - dev_err(dev, - "Invalid memory reference for mmio_base is NULL\n"); - err = -ENODEV; - goto out_error; - } - host = scsi_host_alloc(&ufshcd_driver_template, sizeof(struct ufs_hba)); if (!host) { @@ -2936,9 +2999,40 @@ int ufshcd_init(struct device *dev, struct ufs_hba **hba_handle, hba = shost_priv(host); hba->host = host; hba->dev = dev; + *hba_handle = hba; + +out_error: + return err; +} +EXPORT_SYMBOL(ufshcd_alloc_host); + +/** + * ufshcd_init - Driver initialization routine + * @hba: per-adapter instance + * @mmio_base: base register address + * @irq: Interrupt line of device + * Returns 0 on success, non-zero value on failure + */ +int ufshcd_init(struct ufs_hba *hba, void __iomem *mmio_base, unsigned int irq) +{ + int err; + struct Scsi_Host *host = hba->host; + struct device *dev = hba->dev; + + if (!mmio_base) { + dev_err(hba->dev, + "Invalid memory reference for mmio_base is NULL\n"); + err = -ENODEV; + goto out_error; + } + hba->mmio_base = mmio_base; hba->irq = irq; + err = ufshcd_variant_hba_init(hba); + if (err) + goto out_error; + /* Read capabilities registers */ ufshcd_hba_capabilities(hba); @@ -3010,8 +3104,6 @@ int ufshcd_init(struct device *dev, struct ufs_hba **hba_handle, goto out_remove_scsi_host; } - *hba_handle = hba; - /* Hold auto suspend until async scan completes */ pm_runtime_get_sync(dev); @@ -3023,6 +3115,7 @@ out_remove_scsi_host: scsi_remove_host(hba->host); out_disable: scsi_host_put(host); + ufshcd_variant_hba_exit(hba); out_error: return err; } diff --git a/drivers/scsi/ufs/ufshcd.h b/drivers/scsi/ufs/ufshcd.h index 8f5624e..72acbc7 100644 --- a/drivers/scsi/ufs/ufshcd.h +++ b/drivers/scsi/ufs/ufshcd.h @@ -68,6 +68,8 @@ #define UFSHCD "ufshcd" #define UFSHCD_DRIVER_VERSION "0.2" +struct ufs_hba; + enum dev_cmd_type { DEV_CMD_TYPE_NOP = 0x0, DEV_CMD_TYPE_QUERY = 0x1, @@ -152,6 +154,30 @@ struct ufs_dev_cmd { struct ufs_query query; }; +#define PRE_CHANGE 0 +#define POST_CHANGE 1 +/** + * struct ufs_hba_variant_ops - variant specific callbacks + * @name: variant name + * @init: called when the driver is initialized + * @exit: called to cleanup everything done in init + * @setup_clocks: called before touching any of the controller registers + * @setup_regulators: called before accessing the host controller + * @hce_enable_notify: called before and after HCE enable bit is set to allow + * variant specific Uni-Pro initialization. + * @link_startup_notify: called before and after Link startup is carried out + * to allow variant specific Uni-Pro initialization. + */ +struct ufs_hba_variant_ops { + const char *name; + int (*init)(struct ufs_hba *); + void (*exit)(struct ufs_hba *); + int (*setup_clocks)(struct ufs_hba *, bool); + int (*setup_regulators)(struct ufs_hba *, bool); + int (*hce_enable_notify)(struct ufs_hba *, bool); + int (*link_startup_notify)(struct ufs_hba *, bool); +}; + /** * struct ufs_hba - per adapter private structure * @mmio_base: UFSHCI base register address @@ -171,6 +197,8 @@ struct ufs_dev_cmd { * @nutrs: Transfer Request Queue depth supported by controller * @nutmrs: Task Management Queue depth supported by controller * @ufs_version: UFS Version to which controller complies + * @vops: pointer to variant specific operations + * @priv: pointer to variant specific private data * @irq: Irq number of the controller * @active_uic_cmd: handle of active UIC command * @uic_cmd_mutex: mutex for uic command @@ -217,6 +245,8 @@ struct ufs_hba { int nutrs; int nutmrs; u32 ufs_version; + struct ufs_hba_variant_ops *vops; + void *priv; unsigned int irq; struct uic_command *active_uic_cmd; @@ -253,8 +283,8 @@ struct ufs_hba { #define ufshcd_readl(hba, reg) \ readl((hba)->mmio_base + (reg)) -int ufshcd_init(struct device *, struct ufs_hba ** , void __iomem * , - unsigned int); +int ufshcd_alloc_host(struct device *, struct ufs_hba **); +int ufshcd_init(struct ufs_hba * , void __iomem * , unsigned int); void ufshcd_remove(struct ufs_hba *); /**
Some vendor specific controller versions might need to configure vendor specific - registers, clocks, voltage regulators etc. to initialize the host controller UTP layer and Uni-Pro stack. Provide some common initialization operations that can be used to configure vendor specifics. The methods can be extended in future, for example, for power mode transitions. The operations are vendor/board specific and hence determined with the help of compatible property in device tree. Signed-off-by: Sujit Reddy Thumma <sthumma@codeaurora.org> --- drivers/scsi/ufs/ufshcd-pci.c | 8 +- drivers/scsi/ufs/ufshcd-pltfrm.c | 24 +++++- drivers/scsi/ufs/ufshcd.c | 157 +++++++++++++++++++++++++++++++-------- drivers/scsi/ufs/ufshcd.h | 34 ++++++++- 4 files changed, 187 insertions(+), 36 deletions(-)