[PATCH v4 1/3] driver core: Add ability for arch code to setup pdev_archdata

[PATCH v4 1/3] driver core: Add ability for arch code to setup pdev_archdata

[Kumar Gala]

On some architectures we need to setup pdev_archdata before we add the
device.  Waiting til a bus_notifier is too late since we might need the
pdev_archdata in the bus notifier.  One example is setting up of dma_mask
pointers such that it can be used in a bus_notifier.

We add weak noop version of arch_setup_pdev_archdata() and allow the arch
code to override with access the full definitions of struct device,
struct platform_device, and struct pdev_archdata.

Signed-off-by: Kumar Gala <galak@kernel.crashing.org>
---
v4:
* Move to use a weak function based on comments from Greg
v3:
* Add more comments and add missing call to arch_setup_pdev_archdata in
  platform_device_register
v2:
* Added license, and comments about arch_setup_pdev_archdata()
  per Mike's comments

 drivers/base/platform.c         |   22 ++++++++++++++++++++++
 include/linux/platform_device.h |    1 +
 2 files changed, 23 insertions(+), 0 deletions(-)

diff --git a/drivers/base/platform.c b/drivers/base/platform.c
index 1c291af..060686d 100644
--- a/drivers/base/platform.c
+++ b/drivers/base/platform.c
@@ -32,6 +32,26 @@ struct device platform_bus = {
 EXPORT_SYMBOL_GPL(platform_bus);
 
 /**
+ * arch_setup_pdev_archdata - Allow manipulation of archdata before its used
+ * @dev: platform device
+ *
+ * This is called before platform_device_add() such that any pdev_archdata may
+ * be setup before the platform_notifier is called.  So if a user needs to
+ * manipulate any relevant information in the pdev_archdata they can do:
+ *
+ * 	platform_devic_alloc()
+ * 	... manipulate ...
+ * 	platform_device_add()
+ *
+ * And if they don't care they can just call platform_device_register() and
+ * everything will just work out.
+ */
+void __weak arch_setup_pdev_archdata(struct platform_device *pdev)
+{
+	return ;
+}
+
+/**
  * platform_get_resource - get a resource for a device
  * @dev: platform device
  * @type: resource type
@@ -173,6 +193,7 @@ struct platform_device *platform_device_alloc(const char *name, int id)
 		pa->pdev.id = id;
 		device_initialize(&pa->pdev.dev);
 		pa->pdev.dev.release = platform_device_release;
+		arch_setup_pdev_archdata(&pa->pdev);
 	}
 
 	return pa ? &pa->pdev : NULL;
@@ -334,6 +355,7 @@ EXPORT_SYMBOL_GPL(platform_device_del);
 int platform_device_register(struct platform_device *pdev)
 {
 	device_initialize(&pdev->dev);
+	arch_setup_pdev_archdata(pdev);
 	return platform_device_add(pdev);
 }
 EXPORT_SYMBOL_GPL(platform_device_register);
diff --git a/include/linux/platform_device.h b/include/linux/platform_device.h
index ede1a80..27bb05a 100644
--- a/include/linux/platform_device.h
+++ b/include/linux/platform_device.h
@@ -42,6 +42,7 @@ extern void platform_device_unregister(struct platform_device *);
 extern struct bus_type platform_bus_type;
 extern struct device platform_bus;
 
+extern void arch_setup_pdev_archdata(struct platform_device *);
 extern struct resource *platform_get_resource(struct platform_device *, unsigned int, unsigned int);
 extern int platform_get_irq(struct platform_device *, unsigned int);
 extern struct resource *platform_get_resource_byname(struct platform_device *, unsigned int, const char *);
-- 
1.7.3.4

[PATCH v4 2/3] powerpc: implement arch_setup_pdev_archdata

[Kumar Gala]

We have a long standing issues with platform devices not have a valid
dma_mask pointer.  This hasn't been an issue to date as no platform
device has tried to set its dma_mask value to a non-default value.

Signed-off-by: Kumar Gala <galak@kernel.crashing.org>
---
 arch/powerpc/kernel/setup-common.c |   28 ++++------------------------
 drivers/of/platform.c              |    4 ++--
 2 files changed, 6 insertions(+), 26 deletions(-)

diff --git a/arch/powerpc/kernel/setup-common.c b/arch/powerpc/kernel/setup-common.c
index 79fca26..ea82baa 100644
--- a/arch/powerpc/kernel/setup-common.c
+++ b/arch/powerpc/kernel/setup-common.c
@@ -704,29 +704,9 @@ static int powerpc_debugfs_init(void)
 arch_initcall(powerpc_debugfs_init);
 #endif
 
-static int ppc_dflt_bus_notify(struct notifier_block *nb,
-				unsigned long action, void *data)
+void arch_setup_pdev_archdata(struct platform_device *pdev)
 {
-	struct device *dev = data;
-
-	/* We are only intereted in device addition */
-	if (action != BUS_NOTIFY_ADD_DEVICE)
-		return 0;
-
-	set_dma_ops(dev, &dma_direct_ops);
-
-	return NOTIFY_DONE;
-}
-
-static struct notifier_block ppc_dflt_plat_bus_notifier = {
-	.notifier_call = ppc_dflt_bus_notify,
-	.priority = INT_MAX,
-};
-
-static int __init setup_bus_notifier(void)
-{
-	bus_register_notifier(&platform_bus_type, &ppc_dflt_plat_bus_notifier);
-	return 0;
+	pdev->archdata.dma_mask = DMA_BIT_MASK(32);
+	pdev->dev.dma_mask = &pdev->archdata.dma_mask;
+ 	set_dma_ops(&pdev->dev, &dma_direct_ops);
 }
-
-arch_initcall(setup_bus_notifier);
diff --git a/drivers/of/platform.c b/drivers/of/platform.c
index 63d3cb7..cfc6a79 100644
--- a/drivers/of/platform.c
+++ b/drivers/of/platform.c
@@ -153,7 +153,7 @@ struct platform_device *of_device_alloc(struct device_node *np,
 	}
 
 	dev->dev.of_node = of_node_get(np);
-#if defined(CONFIG_PPC) || defined(CONFIG_MICROBLAZE)
+#if defined(CONFIG_MICROBLAZE)
 	dev->dev.dma_mask = &dev->archdata.dma_mask;
 #endif
 	dev->dev.parent = parent;
@@ -189,7 +189,7 @@ struct platform_device *of_platform_device_create(struct device_node *np,
 	if (!dev)
 		return NULL;
 
-#if defined(CONFIG_PPC) || defined(CONFIG_MICROBLAZE)
+#if defined(CONFIG_MICROBLAZE)
 	dev->archdata.dma_mask = 0xffffffffUL;
 #endif
 	dev->dev.coherent_dma_mask = DMA_BIT_MASK(32);
-- 
1.7.3.4

[PATCH v4 3/3] powerpc: Dont require a dma_ops struct to set dma mask

[Kumar Gala]

The only reason to require a dma_ops struct is to see if it has
implemented set_dma_mask.  If not we can fall back to setting the mask
directly.

This resolves an issue with how to sequence the setting of a DMA mask
for platform devices.  Before we had an issue in that we have no way of
setting the DMA mask before the various low level bus notifiers get
called that might check it (swiotlb).

So now we can do:

	pdev = platform_device_alloc("foobar", 0);
	dma_set_mask(&pdev->dev, DMA_BIT_MASK(37));
	platform_device_add(pdev);

And expect the right thing to happen with the bus notifiers get called
via platform_device_add.

Signed-off-by: Kumar Gala <galak@kernel.crashing.org>
---
 arch/powerpc/kernel/dma.c |    4 +---
 1 files changed, 1 insertions(+), 3 deletions(-)

diff --git a/arch/powerpc/kernel/dma.c b/arch/powerpc/kernel/dma.c
index d238c08..4f0959f 100644
--- a/arch/powerpc/kernel/dma.c
+++ b/arch/powerpc/kernel/dma.c
@@ -161,9 +161,7 @@ int dma_set_mask(struct device *dev, u64 dma_mask)
 
 	if (ppc_md.dma_set_mask)
 		return ppc_md.dma_set_mask(dev, dma_mask);
-	if (unlikely(dma_ops == NULL))
-		return -EIO;
-	if (dma_ops->set_dma_mask != NULL)
+	if ((dma_ops != NULL) && (dma_ops->set_dma_mask != NULL))
 		return dma_ops->set_dma_mask(dev, dma_mask);
 	if (!dev->dma_mask || !dma_supported(dev, dma_mask))
 		return -EIO;
-- 
1.7.3.4

Re: [PATCH v4 1/3] driver core: Add ability for arch code to setup pdev_archdata

[Greg KH]

On Thu, Jul 07, 2011 at 11:00:41PM -0500, Kumar Gala wrote:
> On some architectures we need to setup pdev_archdata before we add the
> device.  Waiting til a bus_notifier is too late since we might need the
> pdev_archdata in the bus notifier.  One example is setting up of dma_mask
> pointers such that it can be used in a bus_notifier.
> 
> We add weak noop version of arch_setup_pdev_archdata() and allow the arch
> code to override with access the full definitions of struct device,
> struct platform_device, and struct pdev_archdata.
> 
> Signed-off-by: Kumar Gala <galak@kernel.crashing.org>
> ---
> v4:
> * Move to use a weak function based on comments from Greg
> v3:
> * Add more comments and add missing call to arch_setup_pdev_archdata in
>   platform_device_register
> v2:
> * Added license, and comments about arch_setup_pdev_archdata()
>   per Mike's comments
> 
>  drivers/base/platform.c         |   22 ++++++++++++++++++++++
>  include/linux/platform_device.h |    1 +
>  2 files changed, 23 insertions(+), 0 deletions(-)
> 
> diff --git a/drivers/base/platform.c b/drivers/base/platform.c
> index 1c291af..060686d 100644
> --- a/drivers/base/platform.c
> +++ b/drivers/base/platform.c
> @@ -32,6 +32,26 @@ struct device platform_bus = {
>  EXPORT_SYMBOL_GPL(platform_bus);
>  
>  /**
> + * arch_setup_pdev_archdata - Allow manipulation of archdata before its used
> + * @dev: platform device
> + *
> + * This is called before platform_device_add() such that any pdev_archdata may
> + * be setup before the platform_notifier is called.  So if a user needs to
> + * manipulate any relevant information in the pdev_archdata they can do:
> + *
> + * 	platform_devic_alloc()
> + * 	... manipulate ...
> + * 	platform_device_add()
> + *
> + * And if they don't care they can just call platform_device_register() and
> + * everything will just work out.
> + */
> +void __weak arch_setup_pdev_archdata(struct platform_device *pdev)
> +{
> +	return ;
> +}

The "return" isn't needed, nor is the extra ' ' there, right?  :)

Change that and you can take this through the ppc tree and add a:
	Acked-by: Greg Kroah-Hartman
to them.

thanks,

greg k-h

Re: [PATCH v4 1/3] driver core: Add ability for arch code to setup pdev_archdata

[Kumar Gala]

On Jul 7, 2011, at 11:22 PM, Greg KH wrote:

> On Thu, Jul 07, 2011 at 11:00:41PM -0500, Kumar Gala wrote:
>> On some architectures we need to setup pdev_archdata before we add =
the
>> device.  Waiting til a bus_notifier is too late since we might need =
the
>> pdev_archdata in the bus notifier.  One example is setting up of =
dma_mask
>> pointers such that it can be used in a bus_notifier.
>>=20
>> We add weak noop version of arch_setup_pdev_archdata() and allow the =
arch
>> code to override with access the full definitions of struct device,
>> struct platform_device, and struct pdev_archdata.
>>=20
>> Signed-off-by: Kumar Gala <galak@kernel.crashing.org>
>> ---
>> v4:
>> * Move to use a weak function based on comments from Greg
>> v3:
>> * Add more comments and add missing call to arch_setup_pdev_archdata =
in
>>  platform_device_register
>> v2:
>> * Added license, and comments about arch_setup_pdev_archdata()
>>  per Mike's comments
>>=20
>> drivers/base/platform.c         |   22 ++++++++++++++++++++++
>> include/linux/platform_device.h |    1 +
>> 2 files changed, 23 insertions(+), 0 deletions(-)
>>=20
>> diff --git a/drivers/base/platform.c b/drivers/base/platform.c
>> index 1c291af..060686d 100644
>> --- a/drivers/base/platform.c
>> +++ b/drivers/base/platform.c
>> @@ -32,6 +32,26 @@ struct device platform_bus =3D {
>> EXPORT_SYMBOL_GPL(platform_bus);
>>=20
>> /**
>> + * arch_setup_pdev_archdata - Allow manipulation of archdata before =
its used
>> + * @dev: platform device
>> + *
>> + * This is called before platform_device_add() such that any =
pdev_archdata may
>> + * be setup before the platform_notifier is called.  So if a user =
needs to
>> + * manipulate any relevant information in the pdev_archdata they can =
do:
>> + *
>> + * 	platform_devic_alloc()
>> + * 	... manipulate ...
>> + * 	platform_device_add()
>> + *
>> + * And if they don't care they can just call =
platform_device_register() and
>> + * everything will just work out.
>> + */
>> +void __weak arch_setup_pdev_archdata(struct platform_device *pdev)
>> +{
>> +	return ;
>> +}
>=20
> The "return" isn't needed, nor is the extra ' ' there, right?  :)

yep, removed the ';' as well ;)

> Change that and you can take this through the ppc tree and add a:
> 	Acked-by: Greg Kroah-Hartman
> to them.

Ack added to all.

thanks

- k=