2013-04-04 10:39:20

by Lee Jones

[permalink] [raw]
Subject: [PATCH 1/2] mfd: db8500-prcmu: Return early if the TCPM cannot be located

Currently we check to see if we obtained the Tightly Coupled Program
Memory (TCPM) base and only execute the code within the check if we
have it. It's more traditional to return early if we don't have it.
This way we can flatten most of the function's code down to a single
tab spacing.

Cc: Samuel Ortiz <[email protected]>
Signed-off-by: Lee Jones <[email protected]>
---
drivers/mfd/db8500-prcmu.c | 40 +++++++++++++++++++++-------------------
1 file changed, 21 insertions(+), 19 deletions(-)

diff --git a/drivers/mfd/db8500-prcmu.c b/drivers/mfd/db8500-prcmu.c
index 21f261b..1c4edbf8 100644
--- a/drivers/mfd/db8500-prcmu.c
+++ b/drivers/mfd/db8500-prcmu.c
@@ -2794,6 +2794,7 @@ static void dbx500_fw_version_init(struct platform_device *pdev,
{
struct resource *res;
void __iomem *tcpm_base;
+ u32 version;

res = platform_get_resource_byname(pdev, IORESOURCE_MEM,
"prcmu-tcpm");
@@ -2803,26 +2804,27 @@ static void dbx500_fw_version_init(struct platform_device *pdev,
return;
}
tcpm_base = ioremap(res->start, resource_size(res));
- if (tcpm_base != NULL) {
- u32 version;
-
- version = readl(tcpm_base + version_offset);
- fw_info.version.project = (version & 0xFF);
- fw_info.version.api_version = (version >> 8) & 0xFF;
- fw_info.version.func_version = (version >> 16) & 0xFF;
- fw_info.version.errata = (version >> 24) & 0xFF;
- strncpy(fw_info.version.project_name,
- fw_project_name(fw_info.version.project),
- PRCMU_FW_PROJECT_NAME_LEN);
- fw_info.valid = true;
- pr_info("PRCMU firmware: %s(%d), version %d.%d.%d\n",
- fw_info.version.project_name,
- fw_info.version.project,
- fw_info.version.api_version,
- fw_info.version.func_version,
- fw_info.version.errata);
- iounmap(tcpm_base);
+ if (!tcpm_base) {
+ dev_err(&pdev->dev, "no prcmu tcpm mem region provided\n");
+ return;
}
+
+ version = readl(tcpm_base + version_offset);
+ fw_info.version.project = (version & 0xFF);
+ fw_info.version.api_version = (version >> 8) & 0xFF;
+ fw_info.version.func_version = (version >> 16) & 0xFF;
+ fw_info.version.errata = (version >> 24) & 0xFF;
+ strncpy(fw_info.version.project_name,
+ fw_project_name(fw_info.version.project),
+ PRCMU_FW_PROJECT_NAME_LEN);
+ fw_info.valid = true;
+ pr_info("PRCMU firmware: %s(%d), version %d.%d.%d\n",
+ fw_info.version.project_name,
+ fw_info.version.project,
+ fw_info.version.api_version,
+ fw_info.version.func_version,
+ fw_info.version.errata);
+ iounmap(tcpm_base);
}

void __init db8500_prcmu_early_init(void)
--
1.7.10.4


2013-04-04 10:39:23

by Lee Jones

[permalink] [raw]
Subject: [PATCH 2/2] mfd: db8500-prcmu: Support platform dependant device selection

The main aim for this cycle is to have the u8540 booting to a
console. However, the u8540 doesn't support all of the u8500
platform devices yet. After this stage is complete we can then
fill in the inadequacies, such as specific clock support at a
later date. To achieve this we're placing devices supported by
all platforms into a common device structure and the remaining
ones into a platform specific one.

Cc: Samuel Ortiz <[email protected]>
Signed-off-by: Lee Jones <[email protected]>
---
drivers/mfd/db8500-prcmu.c | 52 ++++++++++++++++++++++++++++----------------
1 file changed, 33 insertions(+), 19 deletions(-)

diff --git a/drivers/mfd/db8500-prcmu.c b/drivers/mfd/db8500-prcmu.c
index 1c4edbf8..dec94d0 100644
--- a/drivers/mfd/db8500-prcmu.c
+++ b/drivers/mfd/db8500-prcmu.c
@@ -24,6 +24,7 @@
#include <linux/jiffies.h>
#include <linux/bitops.h>
#include <linux/fs.h>
+#include <linux/of.h>
#include <linux/platform_device.h>
#include <linux/uaccess.h>
#include <linux/irqchip/arm-gic.h>
@@ -3107,19 +3108,7 @@ static struct ux500_wdt_data db8500_wdt_pdata = {
.has_28_bits_resolution = true,
};

-static struct mfd_cell db8500_prcmu_devs[] = {
- {
- .name = "db8500-prcmu-regulators",
- .of_compatible = "stericsson,db8500-prcmu-regulator",
- .platform_data = &db8500_regulators,
- .pdata_size = sizeof(db8500_regulators),
- },
- {
- .name = "cpufreq-ux500",
- .of_compatible = "stericsson,cpufreq-ux500",
- .platform_data = &db8500_cpufreq_table,
- .pdata_size = sizeof(db8500_cpufreq_table),
- },
+static struct mfd_cell common_prcmu_devs[] = {
{
.name = "ux500_wdt",
.platform_data = &db8500_wdt_pdata,
@@ -3135,6 +3124,21 @@ static struct mfd_cell db8500_prcmu_devs[] = {
},
};

+static struct mfd_cell db8500_prcmu_devs[] = {
+ {
+ .name = "db8500-prcmu-regulators",
+ .of_compatible = "stericsson,db8500-prcmu-regulator",
+ .platform_data = &db8500_regulators,
+ .pdata_size = sizeof(db8500_regulators),
+ },
+ {
+ .name = "cpufreq-ux500",
+ .of_compatible = "stericsson,cpufreq-ux500",
+ .platform_data = &db8500_cpufreq_table,
+ .pdata_size = sizeof(db8500_cpufreq_table),
+ },
+};
+
static void db8500_prcmu_update_cpufreq(void)
{
if (prcmu_has_arm_maxopp()) {
@@ -3184,10 +3188,10 @@ static int db8500_prcmu_probe(struct platform_device *pdev)

db8500_irq_init(np);

- for (i = 0; i < ARRAY_SIZE(db8500_prcmu_devs); i++) {
- if (!strcmp(db8500_prcmu_devs[i].name, "ab8500-core")) {
- db8500_prcmu_devs[i].platform_data = pdata->ab_platdata;
- db8500_prcmu_devs[i].pdata_size = sizeof(struct ab8500_platform_data);
+ for (i = 0; i < ARRAY_SIZE(common_prcmu_devs); i++) {
+ if (!strcmp(common_prcmu_devs[i].name, "ab8500-core")) {
+ common_prcmu_devs[i].platform_data = pdata->ab_platdata;
+ common_prcmu_devs[i].pdata_size = sizeof(struct ab8500_platform_data);
}
}

@@ -3195,13 +3199,23 @@ static int db8500_prcmu_probe(struct platform_device *pdev)

db8500_prcmu_update_cpufreq();

- err = mfd_add_devices(&pdev->dev, 0, db8500_prcmu_devs,
- ARRAY_SIZE(db8500_prcmu_devs), NULL, 0, NULL);
+ err = mfd_add_devices(&pdev->dev, 0, common_prcmu_devs,
+ ARRAY_SIZE(common_prcmu_devs), NULL, 0, NULL);
if (err) {
pr_err("prcmu: Failed to add subdevices\n");
return err;
}

+ /* TODO: Remove restriction when clk definitions are available. */
+ if (!of_machine_is_compatible("st-ericsson,u8540")) {
+ err = mfd_add_devices(&pdev->dev, 0, db8500_prcmu_devs,
+ ARRAY_SIZE(db8500_prcmu_devs), NULL, 0, NULL);
+ if (err) {
+ pr_err("prcmu: Failed to add subdevices\n");
+ return err;
+ }
+ }
+
pr_info("DB8500 PRCMU initialized\n");

no_irq_return:
--
1.7.10.4

2013-04-09 10:05:16

by Samuel Ortiz

[permalink] [raw]
Subject: Re: [PATCH 1/2] mfd: db8500-prcmu: Return early if the TCPM cannot be located

Hi Lee,

On Thu, Apr 04, 2013 at 11:39:00AM +0100, Lee Jones wrote:
> Currently we check to see if we obtained the Tightly Coupled Program
> Memory (TCPM) base and only execute the code within the check if we
> have it. It's more traditional to return early if we don't have it.
> This way we can flatten most of the function's code down to a single
> tab spacing.
>
> Cc: Samuel Ortiz <[email protected]>
> Signed-off-by: Lee Jones <[email protected]>
> ---
> drivers/mfd/db8500-prcmu.c | 40 +++++++++++++++++++++-------------------
> 1 file changed, 21 insertions(+), 19 deletions(-)
I had to apply this one manually, please check if it was done correctly from
my mfd-next tree.

Cheers,
Samuel.

--
Intel Open Source Technology Centre
http://oss.intel.com/

2013-04-09 10:06:16

by Samuel Ortiz

[permalink] [raw]
Subject: Re: [PATCH 2/2] mfd: db8500-prcmu: Support platform dependant device selection

Hi Lee,

On Thu, Apr 04, 2013 at 11:39:01AM +0100, Lee Jones wrote:
> The main aim for this cycle is to have the u8540 booting to a
> console. However, the u8540 doesn't support all of the u8500
> platform devices yet. After this stage is complete we can then
> fill in the inadequacies, such as specific clock support at a
> later date. To achieve this we're placing devices supported by
> all platforms into a common device structure and the remaining
> ones into a platform specific one.
>
> Cc: Samuel Ortiz <[email protected]>
> Signed-off-by: Lee Jones <[email protected]>
> ---
> drivers/mfd/db8500-prcmu.c | 52 ++++++++++++++++++++++++++++----------------
> 1 file changed, 33 insertions(+), 19 deletions(-)
This one does not apply (not even manually) on top of mfd-next. Would you mind
rebasing it (And adding Linus' ACK) ?

Cheers,
Samuel.

--
Intel Open Source Technology Centre
http://oss.intel.com/

2013-04-09 10:29:56

by Lee Jones

[permalink] [raw]
Subject: Re: [PATCH 1/2] mfd: db8500-prcmu: Return early if the TCPM cannot be located

On Tue, 09 Apr 2013, Samuel Ortiz wrote:

> Hi Lee,
>
> On Thu, Apr 04, 2013 at 11:39:00AM +0100, Lee Jones wrote:
> > Currently we check to see if we obtained the Tightly Coupled Program
> > Memory (TCPM) base and only execute the code within the check if we
> > have it. It's more traditional to return early if we don't have it.
> > This way we can flatten most of the function's code down to a single
> > tab spacing.
> >
> > Cc: Samuel Ortiz <[email protected]>
> > Signed-off-by: Lee Jones <[email protected]>
> > ---
> > drivers/mfd/db8500-prcmu.c | 40 +++++++++++++++++++++-------------------
> > 1 file changed, 21 insertions(+), 19 deletions(-)
> I had to apply this one manually, please check if it was done correctly from
> my mfd-next tree.

Yes, looks good.

I think it was only 'db8500_prcmu_early_init()' and the line numbers
which confused it.

Kind regards,
Lee

--
Lee Jones
Linaro ST-Ericsson Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog

2013-04-09 11:58:30

by Lee Jones

[permalink] [raw]
Subject: Re: [PATCH 2/2] mfd: db8500-prcmu: Support platform dependant device selection

On Thu, 04 Apr 2013, Lee Jones wrote:

> The main aim for this cycle is to have the u8540 booting to a
> console. However, the u8540 doesn't support all of the u8500
> platform devices yet. After this stage is complete we can then
> fill in the inadequacies, such as specific clock support at a
> later date. To achieve this we're placing devices supported by
> all platforms into a common device structure and the remaining
> ones into a platform specific one.
>
> Cc: Samuel Ortiz <[email protected]>
> Signed-off-by: Lee Jones <[email protected]>
> ---
> drivers/mfd/db8500-prcmu.c | 52 ++++++++++++++++++++++++++++----------------
> 1 file changed, 33 insertions(+), 19 deletions(-)
>
> diff --git a/drivers/mfd/db8500-prcmu.c b/drivers/mfd/db8500-prcmu.c
> index 1c4edbf8..dec94d0 100644
> --- a/drivers/mfd/db8500-prcmu.c
> +++ b/drivers/mfd/db8500-prcmu.c
> @@ -24,6 +24,7 @@
> #include <linux/jiffies.h>
> #include <linux/bitops.h>
> #include <linux/fs.h>
> +#include <linux/of.h>
> #include <linux/platform_device.h>
> #include <linux/uaccess.h>
> #include <linux/irqchip/arm-gic.h>
> @@ -3107,19 +3108,7 @@ static struct ux500_wdt_data db8500_wdt_pdata = {
> .has_28_bits_resolution = true,
> };
>
> -static struct mfd_cell db8500_prcmu_devs[] = {
> - {
> - .name = "db8500-prcmu-regulators",
> - .of_compatible = "stericsson,db8500-prcmu-regulator",
> - .platform_data = &db8500_regulators,
> - .pdata_size = sizeof(db8500_regulators),
> - },
> - {
> - .name = "cpufreq-ux500",
> - .of_compatible = "stericsson,cpufreq-ux500",
> - .platform_data = &db8500_cpufreq_table,
> - .pdata_size = sizeof(db8500_cpufreq_table),
> - },
> +static struct mfd_cell common_prcmu_devs[] = {
> {
> .name = "ux500_wdt",
> .platform_data = &db8500_wdt_pdata,
> @@ -3135,6 +3124,21 @@ static struct mfd_cell db8500_prcmu_devs[] = {
> },
> };
>
> +static struct mfd_cell db8500_prcmu_devs[] = {
> + {
> + .name = "db8500-prcmu-regulators",
> + .of_compatible = "stericsson,db8500-prcmu-regulator",
> + .platform_data = &db8500_regulators,
> + .pdata_size = sizeof(db8500_regulators),
> + },
> + {
> + .name = "cpufreq-ux500",
> + .of_compatible = "stericsson,cpufreq-ux500",
> + .platform_data = &db8500_cpufreq_table,
> + .pdata_size = sizeof(db8500_cpufreq_table),
> + },
> +};
> +
> static void db8500_prcmu_update_cpufreq(void)
> {
> if (prcmu_has_arm_maxopp()) {
> @@ -3184,10 +3188,10 @@ static int db8500_prcmu_probe(struct platform_device *pdev)
>
> db8500_irq_init(np);
>
> - for (i = 0; i < ARRAY_SIZE(db8500_prcmu_devs); i++) {
> - if (!strcmp(db8500_prcmu_devs[i].name, "ab8500-core")) {
> - db8500_prcmu_devs[i].platform_data = pdata->ab_platdata;
> - db8500_prcmu_devs[i].pdata_size = sizeof(struct ab8500_platform_data);
> + for (i = 0; i < ARRAY_SIZE(common_prcmu_devs); i++) {
> + if (!strcmp(common_prcmu_devs[i].name, "ab8500-core")) {
> + common_prcmu_devs[i].platform_data = pdata->ab_platdata;
> + common_prcmu_devs[i].pdata_size = sizeof(struct ab8500_platform_data);
> }
> }
>
> @@ -3195,13 +3199,23 @@ static int db8500_prcmu_probe(struct platform_device *pdev)
>
> db8500_prcmu_update_cpufreq();
>
> - err = mfd_add_devices(&pdev->dev, 0, db8500_prcmu_devs,
> - ARRAY_SIZE(db8500_prcmu_devs), NULL, 0, NULL);
> + err = mfd_add_devices(&pdev->dev, 0, common_prcmu_devs,
> + ARRAY_SIZE(common_prcmu_devs), NULL, 0, NULL);
> if (err) {
> pr_err("prcmu: Failed to add subdevices\n");
> return err;
> }
>
> + /* TODO: Remove restriction when clk definitions are available. */
> + if (!of_machine_is_compatible("st-ericsson,u8540")) {
> + err = mfd_add_devices(&pdev->dev, 0, db8500_prcmu_devs,
> + ARRAY_SIZE(db8500_prcmu_devs), NULL, 0, NULL);
> + if (err) {
> + pr_err("prcmu: Failed to add subdevices\n");
> + return err;
> + }
> + }
> +
> pr_info("DB8500 PRCMU initialized\n");
>
> no_irq_return:

Did you see this one Sam?

--
Lee Jones
Linaro ST-Ericsson Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog

2013-04-09 13:01:00

by Samuel Ortiz

[permalink] [raw]
Subject: Re: [PATCH 2/2] mfd: db8500-prcmu: Support platform dependant device selection

In case you did not get this one...

On Tue, Apr 09, 2013 at 12:06:09PM +0200, Samuel Ortiz wrote:
> Hi Lee,
>
> On Thu, Apr 04, 2013 at 11:39:01AM +0100, Lee Jones wrote:
> > The main aim for this cycle is to have the u8540 booting to a
> > console. However, the u8540 doesn't support all of the u8500
> > platform devices yet. After this stage is complete we can then
> > fill in the inadequacies, such as specific clock support at a
> > later date. To achieve this we're placing devices supported by
> > all platforms into a common device structure and the remaining
> > ones into a platform specific one.
> >
> > Cc: Samuel Ortiz <[email protected]>
> > Signed-off-by: Lee Jones <[email protected]>
> > ---
> > drivers/mfd/db8500-prcmu.c | 52 ++++++++++++++++++++++++++++----------------
> > 1 file changed, 33 insertions(+), 19 deletions(-)
> This one does not apply (not even manually) on top of mfd-next. Would you mind
> rebasing it (And adding Linus' ACK) ?
>
> Cheers,
> Samuel.
>
> --
> Intel Open Source Technology Centre
> http://oss.intel.com/

--
Intel Open Source Technology Centre
http://oss.intel.com/

2013-04-09 19:53:06

by Lee Jones

[permalink] [raw]
Subject: [PATCH 2/2 v2] mfd: db8500-prcmu: Support platform dependant device selection

The main aim for this cycle is to have the u8540 booting to a
console. However, the u8540 doesn't support all of the u8500
platform devices yet. After this stage is complete we can then
fill in the inadequacies, such as specific clock support at a
later date. To achieve this we're placing devices supported by
all platforms into a common device structure and the remaining
ones into a platform specific one.

Cc: Samuel Ortiz <[email protected]>
Signed-off-by: Lee Jones <[email protected]>
---
drivers/mfd/db8500-prcmu.c | 32 ++++++++++++++++++++++++--------
1 file changed, 24 insertions(+), 8 deletions(-)

diff --git a/drivers/mfd/db8500-prcmu.c b/drivers/mfd/db8500-prcmu.c
index 25dda8d..319b8ab 100644
--- a/drivers/mfd/db8500-prcmu.c
+++ b/drivers/mfd/db8500-prcmu.c
@@ -24,6 +24,7 @@
#include <linux/jiffies.h>
#include <linux/bitops.h>
#include <linux/fs.h>
+#include <linux/of.h>
#include <linux/platform_device.h>
#include <linux/uaccess.h>
#include <linux/mfd/core.h>
@@ -3067,6 +3068,15 @@ static struct db8500_thsens_platform_data db8500_thsens_data = {
.num_trips = 4,
};

+static struct mfd_cell common_prcmu_devs[] = {
+ {
+ .name = "ux500_wdt",
+ .platform_data = &db8500_wdt_pdata,
+ .pdata_size = sizeof(db8500_wdt_pdata),
+ .id = -1,
+ },
+};
+
static struct mfd_cell db8500_prcmu_devs[] = {
{
.name = "db8500-prcmu-regulators",
@@ -3081,12 +3091,6 @@ static struct mfd_cell db8500_prcmu_devs[] = {
.pdata_size = sizeof(db8500_cpufreq_table),
},
{
- .name = "ux500_wdt",
- .platform_data = &db8500_wdt_pdata,
- .pdata_size = sizeof(db8500_wdt_pdata),
- .id = -1,
- },
- {
.name = "db8500-thermal",
.num_resources = ARRAY_SIZE(db8500_thsens_resources),
.resources = db8500_thsens_resources,
@@ -3175,13 +3179,25 @@ static int db8500_prcmu_probe(struct platform_device *pdev)

db8500_prcmu_update_cpufreq();

- err = mfd_add_devices(&pdev->dev, 0, db8500_prcmu_devs,
- ARRAY_SIZE(db8500_prcmu_devs), NULL, 0, db8500_irq_domain);
+ err = mfd_add_devices(&pdev->dev, 0, common_prcmu_devs,
+ ARRAY_SIZE(common_prcmu_devs), NULL, 0, db8500_irq_domain);
if (err) {
pr_err("prcmu: Failed to add subdevices\n");
return err;
}

+ /* TODO: Remove restriction when clk definitions are available. */
+ if (!of_machine_is_compatible("st-ericsson,u8540")) {
+ err = mfd_add_devices(&pdev->dev, 0, db8500_prcmu_devs,
+ ARRAY_SIZE(db8500_prcmu_devs), NULL, 0,
+ db8500_irq_domain);
+ if (err) {
+ mfd_remove_devices(&pdev->dev);
+ pr_err("prcmu: Failed to add subdevices\n");
+ goto no_irq_return;
+ }
+ }
+
err = db8500_prcmu_register_ab8500(&pdev->dev, pdata->ab_platdata,
pdata->ab_irq);
if (err) {
--
1.7.10.4

2013-04-09 20:55:15

by Samuel Ortiz

[permalink] [raw]
Subject: Re: [PATCH 2/2 v2] mfd: db8500-prcmu: Support platform dependant device selection

Hi Lee,

On Tue, Apr 09, 2013 at 08:52:58PM +0100, Lee Jones wrote:
> The main aim for this cycle is to have the u8540 booting to a
> console. However, the u8540 doesn't support all of the u8500
> platform devices yet. After this stage is complete we can then
> fill in the inadequacies, such as specific clock support at a
> later date. To achieve this we're placing devices supported by
> all platforms into a common device structure and the remaining
> ones into a platform specific one.
>
> Cc: Samuel Ortiz <[email protected]>
> Signed-off-by: Lee Jones <[email protected]>
> ---
> drivers/mfd/db8500-prcmu.c | 32 ++++++++++++++++++++++++--------
> 1 file changed, 24 insertions(+), 8 deletions(-)
Applied and pushed now, thanks.

Cheers,
Samuel.

--
Intel Open Source Technology Centre
http://oss.intel.com/