| Message ID | 20220707013017.26654-1-ansuelsmth@gmail.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | [1/3] hwspinlock: qcom: Add support for mmio usage to sfpb-mutex | expand |
On Wed 06 Jul 20:30 CDT 2022, Christian Marangi wrote: > Allow sfpb-mutex to use mmio in addition to syscon. > Reviewed-by: Bjorn Andersson <bjorn.andersson@linaro.org> > Signed-off-by: Christian Marangi <ansuelsmth@gmail.com> > --- > drivers/hwspinlock/qcom_hwspinlock.c | 32 ++++++++++++++++++++++------ > 1 file changed, 25 insertions(+), 7 deletions(-) > > diff --git a/drivers/hwspinlock/qcom_hwspinlock.c b/drivers/hwspinlock/qcom_hwspinlock.c > index 364710966665..23c913095bd0 100644 > --- a/drivers/hwspinlock/qcom_hwspinlock.c > +++ b/drivers/hwspinlock/qcom_hwspinlock.c > @@ -19,6 +19,11 @@ > #define QCOM_MUTEX_APPS_PROC_ID 1 > #define QCOM_MUTEX_NUM_LOCKS 32 > > +struct qcom_hwspinlock_of_data { > + u32 offset; > + u32 stride; > +}; > + > static int qcom_hwspinlock_trylock(struct hwspinlock *lock) > { > struct regmap_field *field = lock->priv; > @@ -63,9 +68,20 @@ static const struct hwspinlock_ops qcom_hwspinlock_ops = { > .unlock = qcom_hwspinlock_unlock, > }; > > +static const struct qcom_hwspinlock_of_data of_sfpb_mutex = { > + .offset = 0x4, > + .stride = 0x4, > +}; > + > +/* All modern platform has offset 0 and stride of 4k */ > +static const struct qcom_hwspinlock_of_data of_tcsr_mutex = { > + .offset = 0, > + .stride = 0x1000, > +}; > + > static const struct of_device_id qcom_hwspinlock_of_match[] = { > - { .compatible = "qcom,sfpb-mutex" }, > - { .compatible = "qcom,tcsr-mutex" }, > + { .compatible = "qcom,sfpb-mutex", .data = &of_sfpb_mutex }, > + { .compatible = "qcom,tcsr-mutex", .data = &of_tcsr_mutex }, > { } > }; > MODULE_DEVICE_TABLE(of, qcom_hwspinlock_of_match); > @@ -101,7 +117,7 @@ static struct regmap *qcom_hwspinlock_probe_syscon(struct platform_device *pdev, > return regmap; > } > > -static const struct regmap_config tcsr_mutex_config = { > +static const struct regmap_config qcom_hwspinlock_mmio_config = { Any objections against me skipping this rename when applying the patch? Just to keep the patch to the point? Regards, Bjorn > .reg_bits = 32, > .reg_stride = 4, > .val_bits = 32, > @@ -112,18 +128,20 @@ static const struct regmap_config tcsr_mutex_config = { > static struct regmap *qcom_hwspinlock_probe_mmio(struct platform_device *pdev, > u32 *offset, u32 *stride) > { > + const struct qcom_hwspinlock_of_data *data; > struct device *dev = &pdev->dev; > void __iomem *base; > > - /* All modern platform has offset 0 and stride of 4k */ > - *offset = 0; > - *stride = 0x1000; > + data = of_device_get_match_data(dev); > + > + *offset = data->offset; > + *stride = data->stride; > > base = devm_platform_ioremap_resource(pdev, 0); > if (IS_ERR(base)) > return ERR_CAST(base); > > - return devm_regmap_init_mmio(dev, base, &tcsr_mutex_config); > + return devm_regmap_init_mmio(dev, base, &qcom_hwspinlock_mmio_config); > } > > static int qcom_hwspinlock_probe(struct platform_device *pdev) > -- > 2.36.1 >
On Wed, Jul 06, 2022 at 09:53:45PM -0500, Bjorn Andersson wrote: > On Wed 06 Jul 20:30 CDT 2022, Christian Marangi wrote: > > > Allow sfpb-mutex to use mmio in addition to syscon. > > > > Reviewed-by: Bjorn Andersson <bjorn.andersson@linaro.org> > > > Signed-off-by: Christian Marangi <ansuelsmth@gmail.com> > > --- > > drivers/hwspinlock/qcom_hwspinlock.c | 32 ++++++++++++++++++++++------ > > 1 file changed, 25 insertions(+), 7 deletions(-) > > > > diff --git a/drivers/hwspinlock/qcom_hwspinlock.c b/drivers/hwspinlock/qcom_hwspinlock.c > > index 364710966665..23c913095bd0 100644 > > --- a/drivers/hwspinlock/qcom_hwspinlock.c > > +++ b/drivers/hwspinlock/qcom_hwspinlock.c > > @@ -19,6 +19,11 @@ > > #define QCOM_MUTEX_APPS_PROC_ID 1 > > #define QCOM_MUTEX_NUM_LOCKS 32 > > > > +struct qcom_hwspinlock_of_data { > > + u32 offset; > > + u32 stride; > > +}; > > + > > static int qcom_hwspinlock_trylock(struct hwspinlock *lock) > > { > > struct regmap_field *field = lock->priv; > > @@ -63,9 +68,20 @@ static const struct hwspinlock_ops qcom_hwspinlock_ops = { > > .unlock = qcom_hwspinlock_unlock, > > }; > > > > +static const struct qcom_hwspinlock_of_data of_sfpb_mutex = { > > + .offset = 0x4, > > + .stride = 0x4, > > +}; > > + > > +/* All modern platform has offset 0 and stride of 4k */ > > +static const struct qcom_hwspinlock_of_data of_tcsr_mutex = { > > + .offset = 0, > > + .stride = 0x1000, > > +}; > > + > > static const struct of_device_id qcom_hwspinlock_of_match[] = { > > - { .compatible = "qcom,sfpb-mutex" }, > > - { .compatible = "qcom,tcsr-mutex" }, > > + { .compatible = "qcom,sfpb-mutex", .data = &of_sfpb_mutex }, > > + { .compatible = "qcom,tcsr-mutex", .data = &of_tcsr_mutex }, > > { } > > }; > > MODULE_DEVICE_TABLE(of, qcom_hwspinlock_of_match); > > @@ -101,7 +117,7 @@ static struct regmap *qcom_hwspinlock_probe_syscon(struct platform_device *pdev, > > return regmap; > > } > > > > -static const struct regmap_config tcsr_mutex_config = { > > +static const struct regmap_config qcom_hwspinlock_mmio_config = { > > Any objections against me skipping this rename when applying the patch? > Just to keep the patch to the point? > > Regards, > Bjorn > I will send v2 with the name reverted and another fix. > > .reg_bits = 32, > > .reg_stride = 4, > > .val_bits = 32, > > @@ -112,18 +128,20 @@ static const struct regmap_config tcsr_mutex_config = { > > static struct regmap *qcom_hwspinlock_probe_mmio(struct platform_device *pdev, > > u32 *offset, u32 *stride) > > { > > + const struct qcom_hwspinlock_of_data *data; > > struct device *dev = &pdev->dev; > > void __iomem *base; > > > > - /* All modern platform has offset 0 and stride of 4k */ > > - *offset = 0; > > - *stride = 0x1000; > > + data = of_device_get_match_data(dev); > > + > > + *offset = data->offset; > > + *stride = data->stride; > > > > base = devm_platform_ioremap_resource(pdev, 0); > > if (IS_ERR(base)) > > return ERR_CAST(base); > > > > - return devm_regmap_init_mmio(dev, base, &tcsr_mutex_config); > > + return devm_regmap_init_mmio(dev, base, &qcom_hwspinlock_mmio_config); > > } > > > > static int qcom_hwspinlock_probe(struct platform_device *pdev) > > -- > > 2.36.1 > >
On Thu, Jul 7, 2022 at 9:30 AM Christian Marangi <ansuelsmth@gmail.com> wrote: > > Allow sfpb-mutex to use mmio in addition to syscon. > > Signed-off-by: Christian Marangi <ansuelsmth@gmail.com> > --- > drivers/hwspinlock/qcom_hwspinlock.c | 32 ++++++++++++++++++++++------ > 1 file changed, 25 insertions(+), 7 deletions(-) > > diff --git a/drivers/hwspinlock/qcom_hwspinlock.c b/drivers/hwspinlock/qcom_hwspinlock.c > index 364710966665..23c913095bd0 100644 > --- a/drivers/hwspinlock/qcom_hwspinlock.c > +++ b/drivers/hwspinlock/qcom_hwspinlock.c > @@ -19,6 +19,11 @@ > #define QCOM_MUTEX_APPS_PROC_ID 1 > #define QCOM_MUTEX_NUM_LOCKS 32 > > +struct qcom_hwspinlock_of_data { > + u32 offset; > + u32 stride; > +}; > + > static int qcom_hwspinlock_trylock(struct hwspinlock *lock) > { > struct regmap_field *field = lock->priv; > @@ -63,9 +68,20 @@ static const struct hwspinlock_ops qcom_hwspinlock_ops = { > .unlock = qcom_hwspinlock_unlock, > }; > > +static const struct qcom_hwspinlock_of_data of_sfpb_mutex = { > + .offset = 0x4, > + .stride = 0x4, > +}; > + > +/* All modern platform has offset 0 and stride of 4k */ > +static const struct qcom_hwspinlock_of_data of_tcsr_mutex = { > + .offset = 0, > + .stride = 0x1000, > +}; > + > static const struct of_device_id qcom_hwspinlock_of_match[] = { > - { .compatible = "qcom,sfpb-mutex" }, > - { .compatible = "qcom,tcsr-mutex" }, > + { .compatible = "qcom,sfpb-mutex", .data = &of_sfpb_mutex }, > + { .compatible = "qcom,tcsr-mutex", .data = &of_tcsr_mutex }, > { } > }; > MODULE_DEVICE_TABLE(of, qcom_hwspinlock_of_match); > @@ -101,7 +117,7 @@ static struct regmap *qcom_hwspinlock_probe_syscon(struct platform_device *pdev, > return regmap; > } > > -static const struct regmap_config tcsr_mutex_config = { > +static const struct regmap_config qcom_hwspinlock_mmio_config = { > .reg_bits = 32, > .reg_stride = 4, > .val_bits = 32, > @@ -112,18 +128,20 @@ static const struct regmap_config tcsr_mutex_config = { > static struct regmap *qcom_hwspinlock_probe_mmio(struct platform_device *pdev, > u32 *offset, u32 *stride) > { > + const struct qcom_hwspinlock_of_data *data; > struct device *dev = &pdev->dev; > void __iomem *base; > > - /* All modern platform has offset 0 and stride of 4k */ > - *offset = 0; > - *stride = 0x1000; > + data = of_device_get_match_data(dev); Nit: better to validate the return value though this is a rare case. if (!data) return -ENODEV; Otherwise LGTM. Reviewed-by: Baolin Wang <baolin.wang7@gmail.com>
On Thu, Jul 07, 2022 at 07:00:15PM +0800, Baolin Wang wrote: > On Thu, Jul 7, 2022 at 9:30 AM Christian Marangi <ansuelsmth@gmail.com> wrote: > > > > Allow sfpb-mutex to use mmio in addition to syscon. > > > > Signed-off-by: Christian Marangi <ansuelsmth@gmail.com> > > --- > > drivers/hwspinlock/qcom_hwspinlock.c | 32 ++++++++++++++++++++++------ > > 1 file changed, 25 insertions(+), 7 deletions(-) > > > > diff --git a/drivers/hwspinlock/qcom_hwspinlock.c b/drivers/hwspinlock/qcom_hwspinlock.c > > index 364710966665..23c913095bd0 100644 > > --- a/drivers/hwspinlock/qcom_hwspinlock.c > > +++ b/drivers/hwspinlock/qcom_hwspinlock.c > > @@ -19,6 +19,11 @@ > > #define QCOM_MUTEX_APPS_PROC_ID 1 > > #define QCOM_MUTEX_NUM_LOCKS 32 > > > > +struct qcom_hwspinlock_of_data { > > + u32 offset; > > + u32 stride; > > +}; > > + > > static int qcom_hwspinlock_trylock(struct hwspinlock *lock) > > { > > struct regmap_field *field = lock->priv; > > @@ -63,9 +68,20 @@ static const struct hwspinlock_ops qcom_hwspinlock_ops = { > > .unlock = qcom_hwspinlock_unlock, > > }; > > > > +static const struct qcom_hwspinlock_of_data of_sfpb_mutex = { > > + .offset = 0x4, > > + .stride = 0x4, > > +}; > > + > > +/* All modern platform has offset 0 and stride of 4k */ > > +static const struct qcom_hwspinlock_of_data of_tcsr_mutex = { > > + .offset = 0, > > + .stride = 0x1000, > > +}; > > + > > static const struct of_device_id qcom_hwspinlock_of_match[] = { > > - { .compatible = "qcom,sfpb-mutex" }, > > - { .compatible = "qcom,tcsr-mutex" }, > > + { .compatible = "qcom,sfpb-mutex", .data = &of_sfpb_mutex }, > > + { .compatible = "qcom,tcsr-mutex", .data = &of_tcsr_mutex }, > > { } > > }; > > MODULE_DEVICE_TABLE(of, qcom_hwspinlock_of_match); > > @@ -101,7 +117,7 @@ static struct regmap *qcom_hwspinlock_probe_syscon(struct platform_device *pdev, > > return regmap; > > } > > > > -static const struct regmap_config tcsr_mutex_config = { > > +static const struct regmap_config qcom_hwspinlock_mmio_config = { > > .reg_bits = 32, > > .reg_stride = 4, > > .val_bits = 32, > > @@ -112,18 +128,20 @@ static const struct regmap_config tcsr_mutex_config = { > > static struct regmap *qcom_hwspinlock_probe_mmio(struct platform_device *pdev, > > u32 *offset, u32 *stride) > > { > > + const struct qcom_hwspinlock_of_data *data; > > struct device *dev = &pdev->dev; > > void __iomem *base; > > > > - /* All modern platform has offset 0 and stride of 4k */ > > - *offset = 0; > > - *stride = 0x1000; > > + data = of_device_get_match_data(dev); > > Nit: better to validate the return value though this is a rare case. > > if (!data) > return -ENODEV; > Wonder if that can actually happen? Looking at of_device_get_match_data() it can only return the data or NULL if the match data is not defined but considering it should ALWAYS be defined (or the driver can't work). But yhea should be a value to check.
diff --git a/drivers/hwspinlock/qcom_hwspinlock.c b/drivers/hwspinlock/qcom_hwspinlock.c index 364710966665..23c913095bd0 100644 --- a/drivers/hwspinlock/qcom_hwspinlock.c +++ b/drivers/hwspinlock/qcom_hwspinlock.c @@ -19,6 +19,11 @@ #define QCOM_MUTEX_APPS_PROC_ID 1 #define QCOM_MUTEX_NUM_LOCKS 32 +struct qcom_hwspinlock_of_data { + u32 offset; + u32 stride; +}; + static int qcom_hwspinlock_trylock(struct hwspinlock *lock) { struct regmap_field *field = lock->priv; @@ -63,9 +68,20 @@ static const struct hwspinlock_ops qcom_hwspinlock_ops = { .unlock = qcom_hwspinlock_unlock, }; +static const struct qcom_hwspinlock_of_data of_sfpb_mutex = { + .offset = 0x4, + .stride = 0x4, +}; + +/* All modern platform has offset 0 and stride of 4k */ +static const struct qcom_hwspinlock_of_data of_tcsr_mutex = { + .offset = 0, + .stride = 0x1000, +}; + static const struct of_device_id qcom_hwspinlock_of_match[] = { - { .compatible = "qcom,sfpb-mutex" }, - { .compatible = "qcom,tcsr-mutex" }, + { .compatible = "qcom,sfpb-mutex", .data = &of_sfpb_mutex }, + { .compatible = "qcom,tcsr-mutex", .data = &of_tcsr_mutex }, { } }; MODULE_DEVICE_TABLE(of, qcom_hwspinlock_of_match); @@ -101,7 +117,7 @@ static struct regmap *qcom_hwspinlock_probe_syscon(struct platform_device *pdev, return regmap; } -static const struct regmap_config tcsr_mutex_config = { +static const struct regmap_config qcom_hwspinlock_mmio_config = { .reg_bits = 32, .reg_stride = 4, .val_bits = 32, @@ -112,18 +128,20 @@ static const struct regmap_config tcsr_mutex_config = { static struct regmap *qcom_hwspinlock_probe_mmio(struct platform_device *pdev, u32 *offset, u32 *stride) { + const struct qcom_hwspinlock_of_data *data; struct device *dev = &pdev->dev; void __iomem *base; - /* All modern platform has offset 0 and stride of 4k */ - *offset = 0; - *stride = 0x1000; + data = of_device_get_match_data(dev); + + *offset = data->offset; + *stride = data->stride; base = devm_platform_ioremap_resource(pdev, 0); if (IS_ERR(base)) return ERR_CAST(base); - return devm_regmap_init_mmio(dev, base, &tcsr_mutex_config); + return devm_regmap_init_mmio(dev, base, &qcom_hwspinlock_mmio_config); } static int qcom_hwspinlock_probe(struct platform_device *pdev)
Allow sfpb-mutex to use mmio in addition to syscon. Signed-off-by: Christian Marangi <ansuelsmth@gmail.com> --- drivers/hwspinlock/qcom_hwspinlock.c | 32 ++++++++++++++++++++++------ 1 file changed, 25 insertions(+), 7 deletions(-)