mfd updates and proposed changes
cover mfd updates and proposed changes
Updates and changes to mfd
PATCH 1/4 MFD: Use to_platform_device instead of container_of.
Convert mfd_remove_devices_fn() to use to_platform_device() instead of doing container_of().
drivers/mfd/mfd-core.c +1 -2
@@ -99,8 +99,7 @@ EXPORT_SYMBOL(mfd_add_devices);static int mfd_remove_devices_fn (struct device *dev,void *unused) { -platform_device_unregister ( - container_of(dev ,struct platform_device ,dev )); +platform_device_unregister (to_platform_device (dev));return 0 ; }
Re: [patch 1/4] MFD: Use to_platform_device instead of container_of.
2008/7/9 Ben Dooks <[email protected]>:
Quoted 4 lines
Convert
mfd_remove_devices_fn()to useto_platform_device()instead of doingcontainer_of().Signed-off-by Ben Dooks <[email protected]>
Quoted 18 lines
Index:
linux-2.6.26-rc9-next20080709/drivers/mfd/mfd-core.c
===================================================================drivers/mfd/mfd-core.c 2008-07-09 10:14:26.000000000 +0100 renamed from drivers/mfd/mfd-core.c 2008-07-09 10:13:53.000000000 +0100 +1
-3@@ -99,8 +99,7 @@ EXPORT_SYMBOL(mfd_add_devices); static int mfd_remove_devices_fn(struct device *dev, void *unused) { - platform_device_unregister( - container_of(dev, struct platform_device, dev)); + platform_device_unregister(to_platform_device(dev)); return 0; } --
Signature · 2 lines
With best wishes Dmitry
Re: [patch 1/4] MFD: Use to_platform_device instead of container_of.
Quoted 7 lines · Dmitry
2008/7/9 Ben Dooks <[email protected]>:
Quoted 4 lines
Convert
mfd_remove_devices_fn()to useto_platform_device()instead of doingcontainer_of().Signed-off-by Ben Dooks <[email protected]>Acked-by Dmitry Baryshkov <[email protected]>
I'll apply this one after the merge window.
Cheers,
Samuel.
Quoted 24 lines
Quoted 18 lines
Index:
linux-2.6.26-rc9-next20080709/drivers/mfd/mfd-core.c
===================================================================drivers/mfd/mfd-core.c 2008-07-09 10:14:26.000000000 +0100 renamed from drivers/mfd/mfd-core.c 2008-07-09 10:13:53.000000000 +0100 +1
-3@@ -99,8 +99,7 @@ EXPORT_SYMBOL(mfd_add_devices); static int mfd_remove_devices_fn(struct device *dev, void *unused) { - platform_device_unregister( - container_of(dev, struct platform_device, dev)); + platform_device_unregister(to_platform_device(dev)); return 0; } --Signature · 2 lines
With best wishes Dmitry
Samuel Ortiz
AppliedRe: [patch 1/4] MFD: Use to_platform_device instead of container_of.
In reply to Ben DooksQuoted 4 lines · Ben Dooks
Convert
mfd_remove_devices_fn()to useto_platform_device()instead of doingcontainer_of().Signed-off-by Ben Dooks <[email protected]>
Applied to the MFD tree, thanks.
Quoted 16 lines
Index:
linux-2.6.26-rc9-next20080709/drivers/mfd/mfd-core.c
===================================================================drivers/mfd/mfd-core.c +1
-2@@ -99,8 +99,7 @@ EXPORT_SYMBOL(mfd_add_devices);static int mfd_remove_devices_fn (struct device *dev,void *unused) { -platform_device_unregister ( - container_of(dev ,struct platform_device ,dev )); +platform_device_unregister (to_platform_device (dev));return 0 ; }Signature · 1 line
--
PATCH 2/4 MFD: Coding style fixes
In reply to Ben DooksFix some coding style fixes in the mfd core driver.
drivers/mfd/mfd-core.c +7 -8
@@ -16,9 +16,9 @@#include <linux/mfd/core.h> static int mfd_add_device (struct platform_device *parent, -const struct mfd_cell *cell, -struct resource *mem_base, -int irq_base) +const struct mfd_cell *cell, +struct resource *mem_base, +int irq_base) {struct resource res[cell->num_resources];struct platform_device *pdev;
@@ -75,11 +75,10 @@ fail_alloc:return ret; } -int mfd_add_devices ( -struct platform_device *parent, -const struct mfd_cell *cells,int n_devs, -struct resource *mem_base, -int irq_base) +int mfd_add_devices (struct platform_device *parent, +const struct mfd_cell *cells,int n_devs, +struct resource *mem_base, +int irq_base) {int i;int ret =0 ;
include/linux/mfd/core.h +8 -9
@@ -1,5 +1,3 @@ -#ifndef MFD_CORE_H -#define MFD_CORE_H /* * drivers/mfd/mfd-core.h *
@@ -13,6 +11,9 @@* */ +#ifndef MFD_CORE_H +#define MFD_CORE_H +#include <linux/platform_device.h> /*
@@ -38,17 +39,15 @@ struct mfd_cell {const struct resource *resources; }; -static inline struct mfd_cell * -mfd_get_cell (struct platform_device *pdev) +static inline struct mfd_cell *mfd_get_cell (struct platform_device *pdev) {return (struct mfd_cell *)pdev->dev.platform_data; } -extern int mfd_add_devices ( -struct platform_device *parent, -const struct mfd_cell *cells,int n_devs, -struct resource *mem_base, -int irq_base); +extern int mfd_add_devices (struct platform_device *parent, +const struct mfd_cell *cells,int n_devs, +struct resource *mem_base, +int irq_base);extern void mfd_remove_devices (struct platform_device *parent);
Re: [patch 2/4] MFD: Coding style fixes
2008/7/9 Ben Dooks <[email protected]>:
Quoted 3 lines
Fix some coding style fixes in the mfd core driver.
Signed-off-by Ben Dooks <[email protected]>
I don't have a strong feeling about this. As it's pretty much only whitespace changes,
my feelings are closer to NAK. Leaving decision to Samuel or Ian.
Quoted 80 lines
Index:
linux-2.6.26-rc9-next20080709/drivers/mfd/mfd-core.c
===================================================================drivers/mfd/mfd-core.c 2008-07-09 10:44:45.000000000 +0100 renamed from drivers/mfd/mfd-core.c 2008-07-09 10:43:54.000000000 +0100 +7
-8@@ -16,9 +16,9 @@ #include <linux/mfd/core.h> static int mfd_add_device(struct platform_device *parent, - const struct mfd_cell *cell, - struct resource *mem_base, - int irq_base) + const struct mfd_cell *cell, + struct resource *mem_base, + int irq_base) { struct resource res[cell->num_resources]; struct platform_device *pdev;@@ -75,11 +75,10 @@ fail_alloc: return ret; } -int mfd_add_devices( - struct platform_device *parent, - const struct mfd_cell *cells, int n_devs, - struct resource *mem_base, - int irq_base) +int mfd_add_devices(struct platform_device *parent, + const struct mfd_cell *cells, int n_devs, + struct resource *mem_base, + int irq_base) { int i; int ret = 0;Index:
linux-2.6.26-rc9-next20080709/include/linux/mfd/core.h
===================================================================include/linux/mfd/core.h 2008-07-09 10:44:45.000000000 +0100 renamed from include/linux/mfd/core.h 2008-07-09 10:43:54.000000000 +0100 +8
-10@@ -1,5 +1,3 @@ -#ifndef MFD_CORE_H -#define MFD_CORE_H /* * drivers/mfd/mfd-core.h *@@ -13,6 +11,9 @@ * */ +#ifndef MFD_CORE_H +#define MFD_CORE_H + #include <linux/platform_device.h> /*@@ -38,17 +39,15 @@ struct mfd_cell { const struct resource *resources; }; -static inline struct mfd_cell * -mfd_get_cell(struct platform_device *pdev) +static inline struct mfd_cell *mfd_get_cell(struct platform_device *pdev) { return (struct mfd_cell *)pdev->dev.platform_data; } -extern int mfd_add_devices( - struct platform_device *parent, - const struct mfd_cell *cells, int n_devs, - struct resource *mem_base, - int irq_base); +extern int mfd_add_devices(struct platform_device *parent, + const struct mfd_cell *cells, int n_devs, + struct resource *mem_base, + int irq_base); extern void mfd_remove_devices(struct platform_device *parent); --
Signature · 2 lines
With best wishes Dmitry
Re: [patch 2/4] MFD: Coding style fixes
Quoted 8 lines · Dmitry
2008/7/9 Ben Dooks <[email protected]>:
Quoted 3 lines
Fix some coding style fixes in the mfd core driver.
Signed-off-by Ben Dooks <[email protected]>I don't have a strong feeling about this. As it's pretty much only whitespace changes,
my feelings are closer to NAK. Leaving decision to Samuel or Ian.
The coding style is all over the place, this makes it far more readable.
Quoted 86 lines
Quoted 80 lines
Index:
linux-2.6.26-rc9-next20080709/drivers/mfd/mfd-core.c
===================================================================drivers/mfd/mfd-core.c 2008-07-09 10:44:45.000000000 +0100 renamed from drivers/mfd/mfd-core.c 2008-07-09 10:43:54.000000000 +0100 +7
-8@@ -16,9 +16,9 @@ #include <linux/mfd/core.h> static int mfd_add_device(struct platform_device *parent, - const struct mfd_cell *cell, - struct resource *mem_base, - int irq_base) + const struct mfd_cell *cell, + struct resource *mem_base, + int irq_base) { struct resource res[cell->num_resources]; struct platform_device *pdev;@@ -75,11 +75,10 @@ fail_alloc: return ret; } -int mfd_add_devices( - struct platform_device *parent, - const struct mfd_cell *cells, int n_devs, - struct resource *mem_base, - int irq_base) +int mfd_add_devices(struct platform_device *parent, + const struct mfd_cell *cells, int n_devs, + struct resource *mem_base, + int irq_base) { int i; int ret = 0;Index:
linux-2.6.26-rc9-next20080709/include/linux/mfd/core.h
===================================================================include/linux/mfd/core.h 2008-07-09 10:44:45.000000000 +0100 renamed from include/linux/mfd/core.h 2008-07-09 10:43:54.000000000 +0100 +8
-10@@ -1,5 +1,3 @@ -#ifndef MFD_CORE_H -#define MFD_CORE_H /* * drivers/mfd/mfd-core.h *@@ -13,6 +11,9 @@ * */ +#ifndef MFD_CORE_H +#define MFD_CORE_H + #include <linux/platform_device.h> /*@@ -38,17 +39,15 @@ struct mfd_cell { const struct resource *resources; }; -static inline struct mfd_cell * -mfd_get_cell(struct platform_device *pdev) +static inline struct mfd_cell *mfd_get_cell(struct platform_device *pdev) { return (struct mfd_cell *)pdev->dev.platform_data; } -extern int mfd_add_devices( - struct platform_device *parent, - const struct mfd_cell *cells, int n_devs, - struct resource *mem_base, - int irq_base); +extern int mfd_add_devices(struct platform_device *parent, + const struct mfd_cell *cells, int n_devs, + struct resource *mem_base, + int irq_base); extern void mfd_remove_devices(struct platform_device *parent); --Signature · 2 lines
With best wishes Dmitry
Signature · 5 lines
-- Ben Q: What's a light-year? A: One-third less calories than a regular year.
Re: [patch 2/4] MFD: Coding style fixes
Quoted 11 lines · Ben Dooks
Quoted 8 lines · Dmitry
2008/7/9 Ben Dooks <[email protected]>:
Quoted 3 lines
Fix some coding style fixes in the mfd core driver.
Signed-off-by Ben Dooks <[email protected]>I don't have a strong feeling about this. As it's pretty much only whitespace changes,
my feelings are closer to NAK. Leaving decision to Samuel or Ian.The coding style is all over the place, this makes it far more readable.
I agree.
I'll apply it after the merge window as well.
Cheers,
Samuel.
Quoted 94 lines
Quoted 86 lines
Quoted 80 lines
Index:
linux-2.6.26-rc9-next20080709/drivers/mfd/mfd-core.c
===================================================================drivers/mfd/mfd-core.c 2008-07-09 10:44:45.000000000 +0100 renamed from drivers/mfd/mfd-core.c 2008-07-09 10:43:54.000000000 +0100 +7
-8@@ -16,9 +16,9 @@ #include <linux/mfd/core.h> static int mfd_add_device(struct platform_device *parent, - const struct mfd_cell *cell, - struct resource *mem_base, - int irq_base) + const struct mfd_cell *cell, + struct resource *mem_base, + int irq_base) { struct resource res[cell->num_resources]; struct platform_device *pdev;@@ -75,11 +75,10 @@ fail_alloc: return ret; } -int mfd_add_devices( - struct platform_device *parent, - const struct mfd_cell *cells, int n_devs, - struct resource *mem_base, - int irq_base) +int mfd_add_devices(struct platform_device *parent, + const struct mfd_cell *cells, int n_devs, + struct resource *mem_base, + int irq_base) { int i; int ret = 0;Index:
linux-2.6.26-rc9-next20080709/include/linux/mfd/core.h
===================================================================include/linux/mfd/core.h 2008-07-09 10:44:45.000000000 +0100 renamed from include/linux/mfd/core.h 2008-07-09 10:43:54.000000000 +0100 +8
-10@@ -1,5 +1,3 @@ -#ifndef MFD_CORE_H -#define MFD_CORE_H /* * drivers/mfd/mfd-core.h *@@ -13,6 +11,9 @@ * */ +#ifndef MFD_CORE_H +#define MFD_CORE_H + #include <linux/platform_device.h> /*@@ -38,17 +39,15 @@ struct mfd_cell { const struct resource *resources; }; -static inline struct mfd_cell * -mfd_get_cell(struct platform_device *pdev) +static inline struct mfd_cell *mfd_get_cell(struct platform_device *pdev) { return (struct mfd_cell *)pdev->dev.platform_data; } -extern int mfd_add_devices( - struct platform_device *parent, - const struct mfd_cell *cells, int n_devs, - struct resource *mem_base, - int irq_base); +extern int mfd_add_devices(struct platform_device *parent, + const struct mfd_cell *cells, int n_devs, + struct resource *mem_base, + int irq_base); extern void mfd_remove_devices(struct platform_device *parent); --Signature · 2 lines
With best wishes DmitrySignature · 5 lines
-- Ben Q: What's a light-year? A: One-third less calories than a regular year.
Re: [patch 2/4] MFD: Coding style fixes
In reply to DmitryQuoted 5 lines · Dmitry
Quoted 1 line
Signed-off-by Ben Dooks <[email protected]>I don't have a strong feeling about this. As it's pretty much only whitespace changes, my feelings are closer to NAK. Leaving decision to Samuel or Ian.
I like whitespace-fixing patches... so I'm ok with it.
Samuel Ortiz
AppliedRe: [patch 2/4] MFD: Coding style fixes
In reply to Ben DooksQuoted 3 lines · Ben Dooks
Fix some coding style fixes in the mfd core driver.
Signed-off-by Ben Dooks <[email protected]>
Applied as well, thanks.
Cheers,
Samuel.
Quoted 78 lines
Index:
linux-2.6.26-rc9-next20080709/drivers/mfd/mfd-core.c
===================================================================drivers/mfd/mfd-core.c +7
-8@@ -16,9 +16,9 @@#include <linux/mfd/core.h> static int mfd_add_device (struct platform_device *parent, -const struct mfd_cell *cell, -struct resource *mem_base, -int irq_base) +const struct mfd_cell *cell, +struct resource *mem_base, +int irq_base) {struct resource res[cell->num_resources];struct platform_device *pdev;@@ -75,11 +75,10 @@ fail_alloc:return ret; } -int mfd_add_devices ( -struct platform_device *parent, -const struct mfd_cell *cells,int n_devs, -struct resource *mem_base, -int irq_base) +int mfd_add_devices (struct platform_device *parent, +const struct mfd_cell *cells,int n_devs, +struct resource *mem_base, +int irq_base) {int i;int ret =0 ;Index:
linux-2.6.26-rc9-next20080709/include/linux/mfd/core.h
===================================================================include/linux/mfd/core.h +8
-9@@ -1,5 +1,3 @@ -#ifndef MFD_CORE_H -#define MFD_CORE_H /* * drivers/mfd/mfd-core.h * @@ -13,6 +11,9 @@* */ +#ifndef MFD_CORE_H +#define MFD_CORE_H +#include <linux/platform_device.h> /* @@ -38,17 +39,15 @@ struct mfd_cell {const struct resource *resources; }; -static inline struct mfd_cell * -mfd_get_cell (struct platform_device *pdev) +static inline struct mfd_cell *mfd_get_cell (struct platform_device *pdev) {return (struct mfd_cell *)pdev->dev.platform_data; } -extern int mfd_add_devices ( -struct platform_device *parent, -const struct mfd_cell *cells,int n_devs, -struct resource *mem_base, -int irq_base); +extern int mfd_add_devices (struct platform_device *parent, +const struct mfd_cell *cells,int n_devs, +struct resource *mem_base, +int irq_base);extern void mfd_remove_devices (struct platform_device *parent);Signature · 1 line
--
PATCH 3/4 MFD: Remove unnecessary fields if mfd_cell structure.
In reply to Ben DooksThe enable,disable,suspend and resume entry in the struct mfd_cell do not seem to be necessary as they should be handled by the platform driver that the device created has bound to.
include/linux/mfd/core.h +0 -6
@@ -23,12 +26,6 @@*/ struct mfd_cell {const char *name; - -int (*enable)(struct platform_device *dev); -int (*disable)(struct platform_device *dev); -int (*suspend)(struct platform_device *dev); -int (*resume)(struct platform_device *dev); -void *driver_data;/* driver-specific data */ /*
Re: [patch 3/4] MFD: Remove unnecessary fields if mfd_cell structure.
2008/7/9 Ben Dooks <[email protected]>:
Quoted 3 lines
The enable,disable,suspend and resume entry in the struct
mfd_celldo not seem to be necessary as they should be handled by the platform driver that the device created has bound to.
NAK. It's used e.g. by the tmio-nand driver. These fields provide easy way to support
enhanced power management, etc.
Quoted 23 lines
Signed-off-by Ben Dooks <[email protected]>Index:
linux-2.6.26-rc9-next20080709/include/linux/mfd/core.h
===================================================================include/linux/mfd/core.h 2008-07-09 10:45:06.000000000 +0100 renamed from include/linux/mfd/core.h 2008-07-09 10:44:45.000000000 +0100 +0
-7@@ -23,12 +26,6 @@ */ struct mfd_cell { const char *name; - - int (*enable)(struct platform_device *dev); - int (*disable)(struct platform_device *dev); - int (*suspend)(struct platform_device *dev); - int (*resume)(struct platform_device *dev); - void *driver_data; /* driver-specific data */ /* --
Signature · 2 lines
With best wishes Dmitry
Re: [patch 3/4] MFD: Remove unnecessary fields if mfd_cell structure.
Quoted 8 lines · Dmitry
2008/7/9 Ben Dooks <[email protected]>:
Quoted 3 lines
The enable,disable,suspend and resume entry in the struct
mfd_celldo not seem to be necessary as they should be handled by the platform driver that the device created has bound to.NAK. It's used e.g. by the tmio-nand driver. These fields provide easy way to support
enhanced power management, etc.
What enhancements over the standard platform device model do you need?
Quoted 29 lines
Quoted 23 lines
Signed-off-by Ben Dooks <[email protected]>Index:
linux-2.6.26-rc9-next20080709/include/linux/mfd/core.h
===================================================================include/linux/mfd/core.h 2008-07-09 10:45:06.000000000 +0100 renamed from include/linux/mfd/core.h 2008-07-09 10:44:45.000000000 +0100 +0
-7@@ -23,12 +26,6 @@ */ struct mfd_cell { const char *name; - - int (*enable)(struct platform_device *dev); - int (*disable)(struct platform_device *dev); - int (*suspend)(struct platform_device *dev); - int (*resume)(struct platform_device *dev); - void *driver_data; /* driver-specific data */ /* --Signature · 2 lines
With best wishes Dmitry
Signature · 5 lines
-- Ben Q: What's a light-year? A: One-third less calories than a regular year.
Re: [patch 3/4] MFD: Remove unnecessary fields if mfd_cell structure.
2008/7/9 Ben Dooks <[email protected]>:
Quoted 11 lines · Dmitry
Quoted 8 lines
2008/7/9 Ben Dooks <[email protected]>:
Quoted 3 lines
The enable,disable,suspend and resume entry in the struct
mfd_celldo not seem to be necessary as they should be handled by the platform driver that the device created has bound to.NAK. It's used e.g. by the tmio-nand driver. These fields provide easy way to support
enhanced power management, etc.What enhancements over the standard platform device model do you need?
As an example of what we need, please check the tmio-nand driver.
Quoted 39 lines
Quoted 22 lines
Quoted 22 lines
Signed-off-by Ben Dooks <[email protected]>Index:
linux-2.6.26-rc9-next20080709/include/linux/mfd/core.h
===================================================================include/linux/mfd/core.h 2008-07-09 10:45:06.000000000 +0100 renamed from include/linux/mfd/core.h 2008-07-09 10:44:45.000000000 +0100 +0
-6@@ -23,12 +26,6 @@ */ struct mfd_cell { const char *name; - - int (*enable)(struct platform_device *dev); - int (*disable)(struct platform_device *dev); - int (*suspend)(struct platform_device *dev); - int (*resume)(struct platform_device *dev); - void *driver_data; /* driver-specific data */ /*Quoted 8 lines
Quoted 2 lines
--
--
With best wishes
Dmitry--
--
BenQ: What's a light-year? A: One-third less calories than a regular year.
Signature · 2 lines
With best wishes Dmitry
Re: [patch 3/4] MFD: Remove unnecessary fields if mfd_cell structure.
Quoted 7 lines · Dmitry
2008/7/9 Ben Dooks <[email protected]>:
Quoted 6 lines · Dmitry
Quoted 5 lines
2008/7/9 Ben Dooks <[email protected]>:
Quoted 3 lines
The enable,disable,suspend and resume entry in the struct
mfd_celldo not seem to be necessary as they should be handled by the platform driver that the device created has bound to.
Quoted 7 lines
Quoted 5 lines
Quoted 3 lines
NAK. It's used e.g. by the tmio-nand driver. These fields provide easy way to support
enhanced power management, etc.What enhancements over the standard platform device model do you need?
As an example of what we need, please check the tmio-nand driver.
As posted below, the driver on the linux-mtd list does not use this, and further more in [1] the method of getting to the the mfd_cell from a given platform_device has gone which would be used by [2] if it actually touched the mfd_cell information.
[1] http://lists.arm.linux.org.uk/lurker/message/20080708.153450.bb33046d.en.html
[2] http://lists.infradead.org/pipermail/linux-mtd/2008-June/022137.html
Quoted 50 lines
Quoted 39 lines
Quoted 22 lines
Quoted 22 lines
Signed-off-by Ben Dooks <[email protected]>Index:
linux-2.6.26-rc9-next20080709/include/linux/mfd/core.h
===================================================================include/linux/mfd/core.h 2008-07-09 10:45:06.000000000 +0100 renamed from include/linux/mfd/core.h 2008-07-09 10:44:45.000000000 +0100 +0
-6@@ -23,12 +26,6 @@ */ struct mfd_cell { const char *name; - - int (*enable)(struct platform_device *dev); - int (*disable)(struct platform_device *dev); - int (*suspend)(struct platform_device *dev); - int (*resume)(struct platform_device *dev); - void *driver_data; /* driver-specific data */ /*Quoted 8 lines
Quoted 2 lines
--
--
With best wishes
Dmitry--
--
BenQ: What's a light-year? A: One-third less calories than a regular year.Signature · 7 lines
With best wishes Dmitry ------------------------------------------------------------------- List admin: http://lists.arm.linux.org.uk/mailman/listinfo/linux-arm-kernel FAQ: http://www.arm.linux.org.uk/mailinglists/faq.php Etiquette: http://www.arm.linux.org.uk/mailinglists/etiquette.php
Signature · 5 lines
-- Ben Q: What's a light-year? A: One-third less calories than a regular year.
Re: [patch 3/4] MFD: Remove unnecessary fields if mfd_cell structure.
2008/7/9 Ben Dooks <[email protected]>:
Quoted 12 lines · Dmitry
Quoted 11 lines
2008/7/9 Ben Dooks <[email protected]>:
Quoted 10 lines · Dmitry
Quoted 8 lines
2008/7/9 Ben Dooks <[email protected]>:
Quoted 3 lines
The enable,disable,suspend and resume entry in the struct
mfd_celldo not seem to be necessary as they should be handled by the platform driver that the device created has bound to.NAK. It's used e.g. by the tmio-nand driver. These fields provide easy way to support
enhanced power management, etc.
Quoted 8 lines
Quoted 3 lines
Quoted 1 line
What enhancements over the standard platform device model do you need?
As an example of what we need, please check the tmio-nand driver.
As posted below, the driver on the linux-mtd list does not use this, and further more in [1] the method of getting to the the
mfd_cellfrom a givenplatform_devicehas gone which would be used by [2] if it actually touched themfd_cellinformation.
It is used. Go, search for mfd_get_cell.
And yes, it will require a minor change as discussed in the mfd thread yesterday.
Quoted 63 lines
[1] http://lists.arm.linux.org.uk/lurker/message/20080708.153450.bb33046d.en.html
[2] http://lists.infradead.org/pipermail/linux-mtd/2008-June/022137.htmlQuoted 42 lines
Quoted 31 lines
Quoted 22 lines
Quoted 22 lines
Signed-off-by Ben Dooks <[email protected]>Index:
linux-2.6.26-rc9-next20080709/include/linux/mfd/core.h
===================================================================include/linux/mfd/core.h 2008-07-09 10:45:06.000000000 +0100 renamed from include/linux/mfd/core.h 2008-07-09 10:44:45.000000000 +0100 +0
-6@@ -23,12 +26,6 @@ */ struct mfd_cell { const char *name; - - int (*enable)(struct platform_device *dev); - int (*disable)(struct platform_device *dev); - int (*suspend)(struct platform_device *dev); - int (*resume)(struct platform_device *dev); - void *driver_data; /* driver-specific data */ /*Quoted 8 lines
Quoted 2 lines
--
--
With best wishes
DmitryQuoted 8 lines
--
--
BenQ: What's a light-year? A: One-third less calories than a regular year.Quoted 8 lines
--
With best wishes
Dmitry-------------------------------------------------------------------
List admin: http://lists.arm.linux.org.uk/mailman/listinfo/linux-arm-kernel FAQ: http://www.arm.linux.org.uk/mailinglists/faq.php Etiquette: http://www.arm.linux.org.uk/mailinglists/etiquette.php--
--
BenQ: What's a light-year? A: One-third less calories than a regular year.
Signature · 2 lines
With best wishes Dmitry
PATCH 4/4 MFD: Change mfd platform device usage to wrapper platform_device
In reply to Ben DooksThis patch changes the mfd core behaviour to wrapper the platform_device it creates in an struct mfd_device which contains the information about the cell that was created.
1) The creation of the resource list and then passing it to the platform_device_add_resources() causes the allocation of a large array on the stack as well as copying the source data twice (it is copied from the mfd_cell to the temporary array and then copied into the newly allocated array)
2) We can wrapper the platform_device into an mfd_device and use that to do the platform_device and resource allocation in one go to reduce the failiure.
Note, is there actually any reason to pass the sub devices any information about the cell they are created from? The mfd core already makes the appropriate resource adjustments and anything else like clocks should be exported by the clock drivers?
include/linux/mfd/core.h +13 -3
@@ -18,8 +18,6 @@/* * This struct describes the MFD part ("cell"). -* After registration the copy of this structure will become the platform data -* of the resulting platform_device */ struct mfd_cell {const char *name;
@@ -33,9 +31,21 @@ struct mfd_cell {const struct resource *resources; }; +struct mfd_device { +struct platform_device pdev; +struct mfd_cell *cell; +struct resource resources[0 ]; +}; + +static inline struct mfd_device *to_mfd_device (struct device *dev) +{ +struct platform_device *pdev =to_platform_device (dev); + return container_of(pdev, struct mfd_device, pdev); +} +static inline struct mfd_cell *mfd_get_cell (struct platform_device *pdev) { -return (struct mfd_cell *)pdev->dev.platform_data; + return container_of(pdev, struct mfd_device, pdev)->cell; } extern int mfd_add_devices(struct platform_device *parent,
drivers/mfd/mfd-core.c +22 -9
@@ -15,28 +15,41 @@#include <linux/platform_device.h> #include <linux/mfd/core.h> +static void mfd_dev_release (struct device *dev) +{ +struct mfd_device *mdev =to_mfd_device (dev); + +kfree (mdev); +} +static int mfd_add_device (struct platform_device *parent,const struct mfd_cell *cell,struct resource *mem_base,int irq_base) { -struct resource res[cell->num_resources]; +struct resource *res; +struct mfd_device *mdev;struct platform_device *pdev;int ret = -ENOMEM ;int r; - pdev =platform_device_alloc (cell->name, parent->id); -if (!pdev) + mdev =kzalloc (sizeof (struct mfd_device ) + +sizeof (struct resource ) * cell->num_resources, +GFP_KERNEL ); +if (!mdev) goto fail_alloc; - pdev->dev.parent = &parent->dev; + mdev->cell = cell; + mdev->pdev.dev.parent = &parent->dev; - ret =platform_device_add_data (pdev, - cell,sizeof (struct mfd_cell )); -if (ret) - goto fail_device; + pdev = &mdev->pdev; + res = &mdev->resources; + +device_initialise (&pdev->dev); + pdev->id = parent->id; + pdev->name = cell->name; + pdev->release = mfd_device_release; -memzero (res,sizeof (res));for (r =0 ; r < cell->num_resources; r++) { res[r].name = cell->resources[r].name; res[r].flags = cell->resources[r].flags;
Dmitry
NAKRe: [patch 4/4] MFD: Change mfd platform device usage to wrapper platform_device
2008/7/9 Ben Dooks <[email protected]>:
Quoted 14 lines
This patch changes the mfd core behaviour to wrapper the
platform_deviceit creates in an structmfd_devicewhich contains the information about the cell that was created.1) The creation of the resource list and then passing it to the
platform_device_add_resources()causes the allocation of a large array on the stack as well as copying the source data twice (it is copied from themfd_cellto the temporary array and then copied into the newly allocated array)2) We can wrapper the
platform_deviceinto anmfd_deviceand use that to do theplatform_deviceand resource allocation in one go to reduce the failiure.
Quoted 6 lines
Note, is there actually any reason to pass the sub devices any information about the cell they are created from? The mfd core already makes the appropriate resource adjustments and anything else like clocks should be exported by the clock drivers?
Signed-off-by Ben Dooks <[email protected]>
NAK.
0) It was discussed yesterday on the list and the decision was to go in a different way.
I've provided a bit cleaner patch with the same idea, but then we
decided to go in a bit different way.
1) I prefer patch by Mike Rapoport which is more clear and goes in a more correct way.
2) Please examine the tmio-nand driver (was here on the list and on linux-mtd). It uses the mfd_cell
to call hooks from the "host" driver (tc6393xb, more to be added soon).
Quoted 95 lines
Index:
linux-2.6.26-rc9-next20080709/include/linux/mfd/core.h
===================================================================include/linux/mfd/core.h 2008-07-09 11:14:55.000000000 +0100 renamed from include/linux/mfd/core.h 2008-07-09 10:46:23.000000000 +0100 +13
-3@@ -18,8 +18,6 @@ /* * This struct describes the MFD part ("cell"). - * After registration the copy of this structure will become the platform data - * of the resulting platform_device */ struct mfd_cell { const char *name;@@ -33,9 +31,21 @@ struct mfd_cell { const struct resource *resources; }; +struct mfd_device { + struct platform_device pdev; + struct mfd_cell *cell; + struct resource resources[0]; +}; + +static inline struct mfd_device *to_mfd_device(struct device *dev) +{ + struct platform_device *pdev = to_platform_device(dev); + return container_of(pdev, struct mfd_device, pdev); +} + static inline struct mfd_cell *mfd_get_cell(struct platform_device *pdev) { - return (struct mfd_cell *)pdev->dev.platform_data; + return container_of(pdev, struct mfd_device, pdev)->cell; } extern int mfd_add_devices(struct platform_device *parent,Index:
linux-2.6.26-rc9-next20080709/drivers/mfd/mfd-core.c
===================================================================drivers/mfd/mfd-core.c 2008-07-09 11:09:59.000000000 +0100 renamed from drivers/mfd/mfd-core.c 2008-07-09 10:59:54.000000000 +0100 +22
-10@@ -15,28 +15,41 @@ #include <linux/platform_device.h> #include <linux/mfd/core.h> +static void mfd_dev_release(struct device *dev) +{ + struct mfd_device *mdev = to_mfd_device(dev); + + kfree(mdev); +} + static int mfd_add_device(struct platform_device *parent, const struct mfd_cell *cell, struct resource *mem_base, int irq_base) { - struct resource res[cell->num_resources]; + struct resource *res; + struct mfd_device *mdev; struct platform_device *pdev; int ret = -ENOMEM; int r; - pdev = platform_device_alloc(cell->name, parent->id); - if (!pdev) + mdev = kzalloc(sizeof(struct mfd_device) + + sizeof(struct resource) * cell->num_resources, + GFP_KERNEL); + if (!mdev) goto fail_alloc; - pdev->dev.parent = &parent->dev; + mdev->cell = cell; + mdev->pdev.dev.parent = &parent->dev; - ret = platform_device_add_data(pdev, - cell, sizeof(struct mfd_cell)); - if (ret) - goto fail_device; + pdev = &mdev->pdev; + res = &mdev->resources; + + device_initialise(&pdev->dev); + pdev->id = parent->id; + pdev->name = cell->name; + pdev->release = mfd_device_release; - memzero(res, sizeof(res)); for (r = 0; r < cell->num_resources; r++) { res[r].name = cell->resources[r].name; res[r].flags = cell->resources[r].flags; --
Signature · 2 lines
With best wishes Dmitry
Re: [patch 4/4] MFD: Change mfd platform device usage to wrapper platform_device
Quoted 23 lines · Dmitry
2008/7/9 Ben Dooks <[email protected]>:
Quoted 14 lines
This patch changes the mfd core behaviour to wrapper the
platform_deviceit creates in an structmfd_devicewhich contains the information about the cell that was created.1) The creation of the resource list and then passing it to the
platform_device_add_resources()causes the allocation of a large array on the stack as well as copying the source data twice (it is copied from themfd_cellto the temporary array and then copied into the newly allocated array)2) We can wrapper the
platform_deviceinto anmfd_deviceand use that to do theplatform_deviceand resource allocation in one go to reduce the failiure.Quoted 6 lines
Note, is there actually any reason to pass the sub devices any information about the cell they are created from? The mfd core already makes the appropriate resource adjustments and anything else like clocks should be exported by the clock drivers?
Signed-off-by Ben Dooks <[email protected]>
Quoted 7 lines
NAK.
0) It was discussed yesterday on the list and the decision was to go in a different way.
I've provided a bit cleaner patch with the same idea, but then we
decided to go in a bit different way.
1) I prefer patch by Mike Rapoport which is more clear and goes in a more correct way.
How "more correct", whilst the patch by Mike makes the platform data be passed from the cell, there is no longer any way to get from the platform device to the mfd_cell...
The current driver is being inefficent in the way it creates resources on the stack and then calls a routine that does an kalloc/memcpy on the resources.
Quoted 3 lines
2) Please examine the tmio-nand driver (was here on the list and on linux-mtd). It uses the
mfd_cell
to call hooks from the "host" driver (tc6393xb, more to be added soon).
The one posted in [1] does not call these hooks at-all, can ou please explain why these hooks are needed in addition to the ones already available in the platform device driver?
[1] http://lists.infradead.org/pipermail/linux-mtd/2008-June/022137.html
Quoted 101 lines
Quoted 95 lines
Index:
linux-2.6.26-rc9-next20080709/include/linux/mfd/core.h
===================================================================include/linux/mfd/core.h 2008-07-09 11:14:55.000000000 +0100 renamed from include/linux/mfd/core.h 2008-07-09 10:46:23.000000000 +0100 +13
-3@@ -18,8 +18,6 @@ /* * This struct describes the MFD part ("cell"). - * After registration the copy of this structure will become the platform data - * of the resulting platform_device */ struct mfd_cell { const char *name;@@ -33,9 +31,21 @@ struct mfd_cell { const struct resource *resources; }; +struct mfd_device { + struct platform_device pdev; + struct mfd_cell *cell; + struct resource resources[0]; +}; + +static inline struct mfd_device *to_mfd_device(struct device *dev) +{ + struct platform_device *pdev = to_platform_device(dev); + return container_of(pdev, struct mfd_device, pdev); +} + static inline struct mfd_cell *mfd_get_cell(struct platform_device *pdev) { - return (struct mfd_cell *)pdev->dev.platform_data; + return container_of(pdev, struct mfd_device, pdev)->cell; } extern int mfd_add_devices(struct platform_device *parent,Index:
linux-2.6.26-rc9-next20080709/drivers/mfd/mfd-core.c
===================================================================drivers/mfd/mfd-core.c 2008-07-09 11:09:59.000000000 +0100 renamed from drivers/mfd/mfd-core.c 2008-07-09 10:59:54.000000000 +0100 +22
-10@@ -15,28 +15,41 @@ #include <linux/platform_device.h> #include <linux/mfd/core.h> +static void mfd_dev_release(struct device *dev) +{ + struct mfd_device *mdev = to_mfd_device(dev); + + kfree(mdev); +} + static int mfd_add_device(struct platform_device *parent, const struct mfd_cell *cell, struct resource *mem_base, int irq_base) { - struct resource res[cell->num_resources]; + struct resource *res; + struct mfd_device *mdev; struct platform_device *pdev; int ret = -ENOMEM; int r; - pdev = platform_device_alloc(cell->name, parent->id); - if (!pdev) + mdev = kzalloc(sizeof(struct mfd_device) + + sizeof(struct resource) * cell->num_resources, + GFP_KERNEL); + if (!mdev) goto fail_alloc; - pdev->dev.parent = &parent->dev; + mdev->cell = cell; + mdev->pdev.dev.parent = &parent->dev; - ret = platform_device_add_data(pdev, - cell, sizeof(struct mfd_cell)); - if (ret) - goto fail_device; + pdev = &mdev->pdev; + res = &mdev->resources; + + device_initialise(&pdev->dev); + pdev->id = parent->id; + pdev->name = cell->name; + pdev->release = mfd_device_release; - memzero(res, sizeof(res)); for (r = 0; r < cell->num_resources; r++) { res[r].name = cell->resources[r].name; res[r].flags = cell->resources[r].flags; --Signature · 2 lines
With best wishes Dmitry
Signature · 5 lines
-- Ben Q: What's a light-year? A: One-third less calories than a regular year.
Re: [patch 4/4] MFD: Change mfd platform device usage to wrapper platform_device
2008/7/9 Ben Dooks <[email protected]>:
Quoted 32 lines · Dmitry
Quoted 23 lines
2008/7/9 Ben Dooks <[email protected]>:
Quoted 14 lines
This patch changes the mfd core behaviour to wrapper the
platform_deviceit creates in an structmfd_devicewhich contains the information about the cell that was created.1) The creation of the resource list and then passing it to the
platform_device_add_resources()causes the allocation of a large array on the stack as well as copying the source data twice (it is copied from themfd_cellto the temporary array and then copied into the newly allocated array)2) We can wrapper the
platform_deviceinto anmfd_deviceand use that to do theplatform_deviceand resource allocation in one go to reduce the failiure.Quoted 6 lines
Note, is there actually any reason to pass the sub devices any information about the cell they are created from? The mfd core already makes the appropriate resource adjustments and anything else like clocks should be exported by the clock drivers?
Signed-off-by Ben Dooks <[email protected]>Quoted 7 lines
NAK.
0) It was discussed yesterday on the list and the decision was to go in a different way.
I've provided a bit cleaner patch with the same idea, but then we
decided to go in a bit different way.
1) I prefer patch by Mike Rapoport which is more clear and goes in a more correct way.
Quoted 3 lines
How "more correct", whilst the patch by Mike makes the platform data be passed from the cell, there is no longer any way to get from the platform device to the
mfd_cell...
Basically we have two choises for the subdevice driver:
1) it doesn't know about cells at all (e.g. generic-bl, IIRC). Then we are safe to loose that "cell" information
2) If it does use cell information (to get access to hooks), we pass it via platform_data pointer in the mfd_cell and we are ok with it.
Quoted 3 lines
The current driver is being inefficent in the way it creates resources on the stack and then calls a routine that does an kalloc/memcpy on the resources.
I don't see any inefficiency ATM.
Quoted 9 lines
Quoted 3 lines
2) Please examine the tmio-nand driver (was here on the list and on linux-mtd). It uses the
mfd_cell
to call hooks from the "host" driver (tc6393xb, more to be added soon).The one posted in [1] does not call these hooks at-all, can ou please explain why these hooks are needed in addition to the ones already available in the platform device driver?
[1] http://lists.infradead.org/pipermail/linux-mtd/2008-June/022137.html
+
+static int tmio_hw_init(struct platform_device *dev, struct tmio_nand *tmio) +{
+ struct mfd_cell *cell = mfd_get_cell(dev);
+ const struct resource *nfcr = NULL;
+ unsigned long base;
+ int i;
+
+ for (i = 0; i < cell->num_resources; i++)
+ if (!strcmp((cell->resources+i)->name, TMIO_NAND_CONTROL))
+ nfcr = &cell->resources[i];
+
+ if (nfcr == NULL)
+ return -ENOMEM;
+
+ if (cell->enable) {
+ int rc = cell->enable(dev);
+ if (rc)
+ return rc;
+ }
That cell->enable() is necessary to set up the host (in the tc6393xb case to enable buffers)
to enable access to the nand.
Signature · 2 lines
With best wishes Dmitry
Re: [patch 4/4] MFD: Change mfd platform device usage to wrapper platform_device
Quoted 37 lines · Dmitry
2008/7/9 Ben Dooks <[email protected]>:
Quoted 32 lines · Dmitry
Quoted 23 lines
2008/7/9 Ben Dooks <[email protected]>:
Quoted 14 lines
This patch changes the mfd core behaviour to wrapper the
platform_deviceit creates in an structmfd_devicewhich contains the information about the cell that was created.1) The creation of the resource list and then passing it to the
platform_device_add_resources()causes the allocation of a large array on the stack as well as copying the source data twice (it is copied from themfd_cellto the temporary array and then copied into the newly allocated array)2) We can wrapper the
platform_deviceinto anmfd_deviceand use that to do theplatform_deviceand resource allocation in one go to reduce the failiure.Quoted 6 lines
Note, is there actually any reason to pass the sub devices any information about the cell they are created from? The mfd core already makes the appropriate resource adjustments and anything else like clocks should be exported by the clock drivers?
Signed-off-by Ben Dooks <[email protected]>Quoted 7 lines
NAK.
0) It was discussed yesterday on the list and the decision was to go in a different way.
I've provided a bit cleaner patch with the same idea, but then we
decided to go in a bit different way.
1) I prefer patch by Mike Rapoport which is more clear and goes in a more correct way.Quoted 3 lines
How "more correct", whilst the patch by Mike makes the platform data be passed from the cell, there is no longer any way to get from the platform device to the
mfd_cell...
Quoted 5 lines
Basically we have two choises for the subdevice driver:
1) it doesn't know about cells at all (e.g. generic-bl, IIRC). Then we are safe to loose that "cell" information
2) If it does use cell information (to get access to hooks), we pass it viaplatform_datapointer in themfd_celland we are ok with it.
Erm, that is complete non-answer. The driver model and various other parts of the kernel are littered with examples of embedding one structure within another to gain an C++ like object inheritance.
I've supplied an reasonable example of doing this to create an mfd_cell device from an platform_device without creating an large amount of code and improving the efficiency and code-lineage in the process. I do not see how this isn't "correct" or in any way breaing the current linux model of doing things.
Quoted 38 lines
Quoted 4 lines
The current driver is being inefficent in the way it creates resources on the stack and then calls a routine that does an kalloc/memcpy on the resources.
I don't see any inefficiency ATM.
Quoted 9 lines
Quoted 3 lines
2) Please examine the tmio-nand driver (was here on the list and on linux-mtd). It uses the
mfd_cell
to call hooks from the "host" driver (tc6393xb, more to be added soon).The one posted in [1] does not call these hooks at-all, can ou please explain why these hooks are needed in addition to the ones already available in the platform device driver?
[1] http://lists.infradead.org/pipermail/linux-mtd/2008-June/022137.html
(quoted diff) +20
-0+ +static int tmio_hw_init(struct platform_device *dev, struct tmio_nand *tmio) +{ + struct mfd_cell *cell = mfd_get_cell(dev); + const struct resource *nfcr = NULL; + unsigned long base; + int i; + + for (i = 0; i < cell->num_resources; i++) + if (!strcmp((cell->resources+i)->name, TMIO_NAND_CONTROL)) + nfcr = &cell->resources[i]; + + if (nfcr == NULL) + return -ENOMEM; + + if (cell->enable) { + int rc = cell->enable(dev); + if (rc) + return rc; + }
Quoted 3 lines
That
cell->enable()is necessary to set up the host (in the tc6393xb case to enable buffers)
to enable access to the nand.
So, the enable/disable calls might be useful, however is there any reason this could not be handled by the clock framework? The suspend/resume entries where not used, and I belive should not be in here.
As noted before, mfd_get_cell() got dropped by [2]
[2] http://lists.arm.linux.org.uk/lurker/message/20080708.153450.bb33046d.en.html
Signature · 4 lines
Ben Q: What's a light-year? A: One-third less calories than a regular year.
Re: [patch 4/4] MFD: Change mfd platform device usage to wrapper platform_device
2008/7/9 Ben Dooks <[email protected]>:
Quoted 48 lines · Dmitry
Quoted 37 lines
2008/7/9 Ben Dooks <[email protected]>:
Quoted 32 lines · Dmitry
Quoted 23 lines
2008/7/9 Ben Dooks <[email protected]>:
Quoted 14 lines
This patch changes the mfd core behaviour to wrapper the
platform_deviceit creates in an structmfd_devicewhich contains the information about the cell that was created.1) The creation of the resource list and then passing it to the
platform_device_add_resources()causes the allocation of a large array on the stack as well as copying the source data twice (it is copied from themfd_cellto the temporary array and then copied into the newly allocated array)2) We can wrapper the
platform_deviceinto anmfd_deviceand use that to do theplatform_deviceand resource allocation in one go to reduce the failiure.Quoted 6 lines
Note, is there actually any reason to pass the sub devices any information about the cell they are created from? The mfd core already makes the appropriate resource adjustments and anything else like clocks should be exported by the clock drivers?
Signed-off-by Ben Dooks <[email protected]>Quoted 7 lines
NAK.
0) It was discussed yesterday on the list and the decision was to go in a different way.
I've provided a bit cleaner patch with the same idea, but then we
decided to go in a bit different way.
1) I prefer patch by Mike Rapoport which is more clear and goes in a more correct way.Quoted 3 lines
How "more correct", whilst the patch by Mike makes the platform data be passed from the cell, there is no longer any way to get from the platform device to the
mfd_cell...Quoted 5 lines
Basically we have two choises for the subdevice driver:
1) it doesn't know about cells at all (e.g. generic-bl, IIRC). Then we are safe to loose that "cell" information
2) If it does use cell information (to get access to hooks), we pass it viaplatform_datapointer in themfd_celland we are ok with it.Erm, that is complete non-answer. The driver model and various other parts of the kernel are littered with examples of embedding one structure within another to gain an C++ like object inheritance.
Quoted 5 lines
I've supplied an reasonable example of doing this to create an
mfd_celldevice from anplatform_devicewithout creating an large amount of code and improving the efficiency and code-lineage in the process. I do not see how this isn't "correct" or in any way breaing the current linux model of doing things.
It isn't breaking it. OK. I'm leaving the decision to the MFD or ARM maintainers.
And BTW, nearly the same patch was sent yesterday by me[1]. Is it an independant work, or did you miss my sign-off?
[1]: http://permalink.gmane.org/gmane.linux.ports.arm.kernel/44142
Quoted 39 lines
Quoted 39 lines
Quoted 5 lines
The current driver is being inefficent in the way it creates resources on the stack and then calls a routine that does an kalloc/memcpy on the resources.
I don't see any inefficiency ATM.
Quoted 9 lines
Quoted 3 lines
2) Please examine the tmio-nand driver (was here on the list and on linux-mtd). It uses the
mfd_cell
to call hooks from the "host" driver (tc6393xb, more to be added soon).The one posted in [1] does not call these hooks at-all, can ou please explain why these hooks are needed in addition to the ones already available in the platform device driver?
[1] http://lists.infradead.org/pipermail/linux-mtd/2008-June/022137.html
(quoted diff) +20
-0+ +static int tmio_hw_init(struct platform_device *dev, struct tmio_nand *tmio) +{ + struct mfd_cell *cell = mfd_get_cell(dev); + const struct resource *nfcr = NULL; + unsigned long base; + int i; + + for (i = 0; i < cell->num_resources; i++) + if (!strcmp((cell->resources+i)->name, TMIO_NAND_CONTROL)) + nfcr = &cell->resources[i]; + + if (nfcr == NULL) + return -ENOMEM; + + if (cell->enable) { + int rc = cell->enable(dev); + if (rc) + return rc; + }
Quoted 7 lines
Quoted 3 lines
That
cell->enable()is necessary to set up the host (in the tc6393xb case to enable buffers)
to enable access to the nand.So, the enable/disable calls might be useful, however is there any reason this could not be handled by the clock framework? The suspend/resume entries where not used, and I belive should not be in here.
They should be here for exactly the same reason. They are used by the drivers that will be submitted later. E.g. OHCI driver needs such suspend/resume handling.
Quoted 3 lines
As noted before,
mfd_get_cell()got dropped by [2][2] http://lists.arm.linux.org.uk/lurker/message/20080708.153450.bb33046d.en.html
Yes, and as I said before it will need some small modifications.
Signature · 2 lines
With best wishes Dmitry
Re: [patch 4/4] MFD: Change mfd platform device usage to wrapper platform_device
Quoted 55 lines · Dmitry
2008/7/9 Ben Dooks <[email protected]>:
Quoted 48 lines · Dmitry
Quoted 37 lines
2008/7/9 Ben Dooks <[email protected]>:
Quoted 32 lines · Dmitry
Quoted 23 lines
2008/7/9 Ben Dooks <[email protected]>:
Quoted 14 lines
This patch changes the mfd core behaviour to wrapper the
platform_deviceit creates in an structmfd_devicewhich contains the information about the cell that was created.1) The creation of the resource list and then passing it to the
platform_device_add_resources()causes the allocation of a large array on the stack as well as copying the source data twice (it is copied from themfd_cellto the temporary array and then copied into the newly allocated array)2) We can wrapper the
platform_deviceinto anmfd_deviceand use that to do theplatform_deviceand resource allocation in one go to reduce the failiure.Quoted 6 lines
Note, is there actually any reason to pass the sub devices any information about the cell they are created from? The mfd core already makes the appropriate resource adjustments and anything else like clocks should be exported by the clock drivers?
Signed-off-by Ben Dooks <[email protected]>Quoted 7 lines
NAK.
0) It was discussed yesterday on the list and the decision was to go in a different way.
I've provided a bit cleaner patch with the same idea, but then we
decided to go in a bit different way.
1) I prefer patch by Mike Rapoport which is more clear and goes in a more correct way.Quoted 3 lines
How "more correct", whilst the patch by Mike makes the platform data be passed from the cell, there is no longer any way to get from the platform device to the
mfd_cell...Quoted 5 lines
Basically we have two choises for the subdevice driver:
1) it doesn't know about cells at all (e.g. generic-bl, IIRC). Then we are safe to loose that "cell" information
2) If it does use cell information (to get access to hooks), we pass it viaplatform_datapointer in themfd_celland we are ok with it.Erm, that is complete non-answer. The driver model and various other parts of the kernel are littered with examples of embedding one structure within another to gain an C++ like object inheritance.
Quoted 5 lines
I've supplied an reasonable example of doing this to create an
mfd_celldevice from anplatform_devicewithout creating an large amount of code and improving the efficiency and code-lineage in the process. I do not see how this isn't "correct" or in any way breaing the current linux model of doing things.
Quoted 6 lines
It isn't breaking it. OK. I'm leaving the decision to the MFD or ARM maintainers.
And BTW, nearly the same patch was sent yesterday by me[1]. Is it an independant work, or did you miss my sign-off?[1]: http://permalink.gmane.org/gmane.linux.ports.arm.kernel/44142
Hmm, thanks, completely missed because it has a completely un-related looking title.
Quoted 47 lines
Quoted 39 lines
Quoted 39 lines
Quoted 5 lines
The current driver is being inefficent in the way it creates resources on the stack and then calls a routine that does an kalloc/memcpy on the resources.
I don't see any inefficiency ATM.
Quoted 9 lines
Quoted 3 lines
2) Please examine the tmio-nand driver (was here on the list and on linux-mtd). It uses the
mfd_cell
to call hooks from the "host" driver (tc6393xb, more to be added soon).The one posted in [1] does not call these hooks at-all, can ou please explain why these hooks are needed in addition to the ones already available in the platform device driver?
[1] http://lists.infradead.org/pipermail/linux-mtd/2008-June/022137.html
(quoted diff) +20
-0+ +static int tmio_hw_init(struct platform_device *dev, struct tmio_nand *tmio) +{ + struct mfd_cell *cell = mfd_get_cell(dev); + const struct resource *nfcr = NULL; + unsigned long base; + int i; + + for (i = 0; i < cell->num_resources; i++) + if (!strcmp((cell->resources+i)->name, TMIO_NAND_CONTROL)) + nfcr = &cell->resources[i]; + + if (nfcr == NULL) + return -ENOMEM; + + if (cell->enable) { + int rc = cell->enable(dev); + if (rc) + return rc; + }Quoted 7 lines
Quoted 3 lines
That
cell->enable()is necessary to set up the host (in the tc6393xb case to enable buffers)
to enable access to the nand.So, the enable/disable calls might be useful, however is there any reason this could not be handled by the clock framework? The suspend/resume entries where not used, and I belive should not be in here.
Quoted 3 lines
They should be here for exactly the same reason. They are used by the drivers that will be submitted later. E.g. OHCI driver needs such suspend/resume handling.
No, you don't understand. I'll make a rather explicit point about the very clever way the device tree works since the devices are registered with their parent device set.
In the suspend, all sub devices are suspended via their platform_driver.suspend method before the parent device's suspend method is called. When resuming, the parent is resumed before calling the children's platform_driver.resume methods.
Quoted 14 lines
Quoted 3 lines
As noted before,
mfd_get_cell()got dropped by [2][2] http://lists.arm.linux.org.uk/lurker/message/20080708.153450.bb33046d.en.html
Yes, and as I said before it will need some small modifications.
Signature · 7 lines
With best wishes Dmitry ------------------------------------------------------------------- List admin: http://lists.arm.linux.org.uk/mailman/listinfo/linux-arm-kernel FAQ: http://www.arm.linux.org.uk/mailinglists/faq.php Etiquette: http://www.arm.linux.org.uk/mailinglists/etiquette.php
Signature · 4 lines
Ben Q: What's a light-year? A: One-third less calories than a regular year.
Re: [patch 4/4] MFD: Change mfd platform device usage to wrapper platform_device
2008/7/9 Ben Dooks <[email protected]>:
Quoted 117 lines · Dmitry
Quoted 55 lines
2008/7/9 Ben Dooks <[email protected]>:
Quoted 48 lines · Dmitry
Quoted 37 lines
2008/7/9 Ben Dooks <[email protected]>:
Quoted 32 lines · Dmitry
Quoted 23 lines
2008/7/9 Ben Dooks <[email protected]>:
Quoted 14 lines
This patch changes the mfd core behaviour to wrapper the
platform_deviceit creates in an structmfd_devicewhich contains the information about the cell that was created.1) The creation of the resource list and then passing it to the
platform_device_add_resources()causes the allocation of a large array on the stack as well as copying the source data twice (it is copied from themfd_cellto the temporary array and then copied into the newly allocated array)2) We can wrapper the
platform_deviceinto anmfd_deviceand use that to do theplatform_deviceand resource allocation in one go to reduce the failiure.Quoted 6 lines
Note, is there actually any reason to pass the sub devices any information about the cell they are created from? The mfd core already makes the appropriate resource adjustments and anything else like clocks should be exported by the clock drivers?
Signed-off-by Ben Dooks <[email protected]>Quoted 7 lines
NAK.
0) It was discussed yesterday on the list and the decision was to go in a different way.
I've provided a bit cleaner patch with the same idea, but then we
decided to go in a bit different way.
1) I prefer patch by Mike Rapoport which is more clear and goes in a more correct way.Quoted 3 lines
How "more correct", whilst the patch by Mike makes the platform data be passed from the cell, there is no longer any way to get from the platform device to the
mfd_cell...Quoted 5 lines
Basically we have two choises for the subdevice driver:
1) it doesn't know about cells at all (e.g. generic-bl, IIRC). Then we are safe to loose that "cell" information
2) If it does use cell information (to get access to hooks), we pass it viaplatform_datapointer in themfd_celland we are ok with it.Erm, that is complete non-answer. The driver model and various other parts of the kernel are littered with examples of embedding one structure within another to gain an C++ like object inheritance.
Quoted 5 lines
I've supplied an reasonable example of doing this to create an
mfd_celldevice from anplatform_devicewithout creating an large amount of code and improving the efficiency and code-lineage in the process. I do not see how this isn't "correct" or in any way breaing the current linux model of doing things.Quoted 6 lines
It isn't breaking it. OK. I'm leaving the decision to the MFD or ARM maintainers.
And BTW, nearly the same patch was sent yesterday by me[1]. Is it an independant work, or did you miss my sign-off?[1]: http://permalink.gmane.org/gmane.linux.ports.arm.kernel/44142
Hmm, thanks, completely missed because it has a completely un-related looking title.
Quoted 47 lines
Quoted 39 lines
Quoted 39 lines
Quoted 5 lines
The current driver is being inefficent in the way it creates resources on the stack and then calls a routine that does an kalloc/memcpy on the resources.
I don't see any inefficiency ATM.
Quoted 9 lines
Quoted 3 lines
2) Please examine the tmio-nand driver (was here on the list and on linux-mtd). It uses the
mfd_cell
to call hooks from the "host" driver (tc6393xb, more to be added soon).The one posted in [1] does not call these hooks at-all, can ou please explain why these hooks are needed in addition to the ones already available in the platform device driver?
[1] http://lists.infradead.org/pipermail/linux-mtd/2008-June/022137.html
(quoted diff) +20
-0+ +static int tmio_hw_init(struct platform_device *dev, struct tmio_nand *tmio) +{ + struct mfd_cell *cell = mfd_get_cell(dev); + const struct resource *nfcr = NULL; + unsigned long base; + int i; + + for (i = 0; i < cell->num_resources; i++) + if (!strcmp((cell->resources+i)->name, TMIO_NAND_CONTROL)) + nfcr = &cell->resources[i]; + + if (nfcr == NULL) + return -ENOMEM; + + if (cell->enable) { + int rc = cell->enable(dev); + if (rc) + return rc; + }Quoted 7 lines
Quoted 3 lines
That
cell->enable()is necessary to set up the host (in the tc6393xb case to enable buffers)
to enable access to the nand.So, the enable/disable calls might be useful, however is there any reason this could not be handled by the clock framework? The suspend/resume entries where not used, and I belive should not be in here.
Quoted 3 lines
They should be here for exactly the same reason. They are used by the drivers that will be submitted later. E.g. OHCI driver needs such suspend/resume handling.
Quoted 8 lines
No, you don't understand. I'll make a rather explicit point about the very clever way the device tree works since the devices are registered with their parent device set.
In the suspend, all sub devices are suspended via their platform_driver.suspend method before the parent device's suspend method is called. When resuming, the parent is resumed before calling the children's platform_driver.resume methods.
Suspending of sub-devices is handled in two places:
1) suspend the state of subdevice (e.g. ohci stores some info) and then
2) sub-device host suspends/disables the cell (e.g. clocks, power, etc). These steps depend completely on the MFD-device, not only on the sub-device.
These two-stage power management is represented by the suspend/resume hooks in the mfd_cell. However, I think we will be able to drop the suspend/resume when/if
generic clocks and voltage regulators frameworks get merged.
Quoted 22 lines
Quoted 6 lines
Quoted 3 lines
As noted before,
mfd_get_cell()got dropped by [2][2] http://lists.arm.linux.org.uk/lurker/message/20080708.153450.bb33046d.en.html
Yes, and as I said before it will need some small modifications.
Quoted 8 lines
--
With best wishes
Dmitry-------------------------------------------------------------------
List admin: http://lists.arm.linux.org.uk/mailman/listinfo/linux-arm-kernel FAQ: http://www.arm.linux.org.uk/mailinglists/faq.php Etiquette: http://www.arm.linux.org.uk/mailinglists/etiquette.php--
BenQ: What's a light-year? A: One-third less calories than a regular year.
Signature · 2 lines
With best wishes Dmitry
Re: [patch 4/4] MFD: Change mfd platform device usage to wrapper platform_device
In reply to Ben DooksQuoted 8 lines · Ben Dooks
Quoted 1 line
They should be here for exactly the same reason. They are used by
the drivers
Quoted 2 lines
that will be submitted later. E.g. OHCI driver needs such suspend/resume handling.
No, you don't understand. I'll make a rather explicit point about the very clever way the device tree works since the devices are registered with their parent device set.
Actually I misthought here too.
The problem comes when the subdevices arent *quite* truely independant of their core, and thus need to ask the core to turn off power / clokcs / etc. for them
they cant just do it themselves because the subdevices may be used on more than one core that does this hanling in different ways (eg. T7L and TC6393XB handle the 32KHz clock completely differently. on tc6393xb, theres a clock gate on the MFD core chip and on t7l66xb the clock has to be handled right back at the platform layer, in board specific code because the core has no gate, and the clock is fed to it from an external pin.
Thats one example - there are others, not all clocks.
Re: [patch 4/4] MFD: Change mfd platform device usage to wrapper platform_device
Quoted 10 lines · ian
Quoted 8 lines · Ben Dooks
Quoted 1 line
They should be here for exactly the same reason. They are used by
the drivers
Quoted 2 lines
that will be submitted later. E.g. OHCI driver needs such suspend/resume handling.
No, you don't understand. I'll make a rather explicit point about the very clever way the device tree works since the devices are registered with their parent device set.
Quoted 5 lines
Actually I misthought here too.
The problem comes when the subdevices arent *quite* truely independant of their core, and thus need to ask the core to turn off power / clokcs / etc. for them
I agree "power" and "etc." are issues. Clocks should be handled by the clock API just fine.
Quoted 3 lines
they cant just do it themselves because the subdevices may be used on more than one core that does this hanling in different ways (eg. T7L and TC6393XB handle the 32KHz clock completely differently.
That shouln't matter with generic clocks. If they clk_get(&mfd_cell->pdev.dev, "my_clk_input"), that should be dispatched to the correct MFD clock regardless of the actual chip.
Quoted 5 lines
on tc6393xb,
theres a clock gate on the MFD core chip and on t7l66xb the clock has to be handled right back at the platform layer, in board specific code because the core has no gate, and the clock is fed to it from an external pin.
Still, that could be done inside the MFD driver's custom clock enable/disable methods.
Quoted 8 lines
Thats one example - there are others, not all clocks.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [patch 4/4] MFD: Change mfd platform device usage to wrapper platform_device
Quoted 5 lines · pHilipp Zabel
Quoted 1 line · ian
On Wed, 2008-07-09 at 13:07 +0100, Ben Dooks wrote:
I agree "power" and "etc." are issues. Clocks should be handled by the clock API just fine.
When its merged.
Quoted 7 lines
Quoted 3 lines
they cant just do it themselves because the subdevices may be used on more than one core that does this hanling in different ways (eg. T7L and TC6393XB handle the 32KHz clock completely differently.
That shouln't matter with generic clocks. If they
clk_get(&mfd_cell->pdev.dev, "my_clk_input"), that should be dispatched to the correct MFD clock regardless of the actual chip.
I agree but its not merged yet.
Until it is, we should stick with the enable / disable methods.
Also, suspend / resume for TC* need careful sequencing of the GPIOs attached to #PCLR and #SUSPEND (get it wrong and you reset the chip instead of sleep it)
:-)
Re: [patch 4/4] MFD: Change mfd platform device usage to wrapper platform_device
In reply to DmitryQuoted 16 lines · Dmitry
Quoted 10 lines
Quoted 5 lines
Basically we have two choises for the subdevice driver:
1) it doesn't know about cells at all (e.g. generic-bl, IIRC). Then we are safe to loose that "cell" information
2) If it does use cell information (to get access to hooks), we pass it viaplatform_datapointer in themfd_celland we are ok with it.Erm, that is complete non-answer. The driver model and various other parts of the kernel are littered with examples of embedding one structure within another to gain an C++ like object inheritance.
Quoted 5 lines
I've supplied an reasonable example of doing this to create an
mfd_celldevice from anplatform_devicewithout creating an large amount of code and improving the efficiency and code-lineage in the process. I do not see how this isn't "correct" or in any way breaing the current linux model of doing things.
Quoted 6 lines
It isn't breaking it. OK. I'm leaving the decision to the MFD or ARM maintainers.
And BTW, nearly the same patch was sent yesterday by me[1]. Is it an independant work, or did you miss my sign-off?[1]: http://permalink.gmane.org/gmane.linux.ports.arm.kernel/44142
As Russel pointed out, both patches are broken as long as we support a modular mfd-core (and there's no reason why we shouldnt). So, so far, Mike's patch is the best candidate, even though the idea of wrapping the platform device into an mfd_device sounded neater at first.
And again, this is definitely post merge window material.
Cheers,
Samuel.
Quoted 65 lines
Quoted 39 lines
Quoted 39 lines
Quoted 5 lines
The current driver is being inefficent in the way it creates resources on the stack and then calls a routine that does an kalloc/memcpy on the resources.
I don't see any inefficiency ATM.
Quoted 9 lines
Quoted 3 lines
2) Please examine the tmio-nand driver (was here on the list and on linux-mtd). It uses the
mfd_cell
to call hooks from the "host" driver (tc6393xb, more to be added soon).The one posted in [1] does not call these hooks at-all, can ou please explain why these hooks are needed in addition to the ones already available in the platform device driver?
[1] http://lists.infradead.org/pipermail/linux-mtd/2008-June/022137.html
(quoted diff) +20
-0+ +static int tmio_hw_init(struct platform_device *dev, struct tmio_nand *tmio) +{ + struct mfd_cell *cell = mfd_get_cell(dev); + const struct resource *nfcr = NULL; + unsigned long base; + int i; + + for (i = 0; i < cell->num_resources; i++) + if (!strcmp((cell->resources+i)->name, TMIO_NAND_CONTROL)) + nfcr = &cell->resources[i]; + + if (nfcr == NULL) + return -ENOMEM; + + if (cell->enable) { + int rc = cell->enable(dev); + if (rc) + return rc; + }Quoted 7 lines
Quoted 3 lines
That
cell->enable()is necessary to set up the host (in the tc6393xb case to enable buffers)
to enable access to the nand.So, the enable/disable calls might be useful, however is there any reason this could not be handled by the clock framework? The suspend/resume entries where not used, and I belive should not be in here.
They should be here for exactly the same reason. They are used by the drivers that will be submitted later. E.g. OHCI driver needs such suspend/resume handling.
Quoted 3 lines
As noted before,
mfd_get_cell()got dropped by [2][2] http://lists.arm.linux.org.uk/lurker/message/20080708.153450.bb33046d.en.html
Yes, and as I said before it will need some small modifications.
Signature · 7 lines
With best wishes Dmitry ------------------------------------------------------------------- List admin: http://lists.arm.linux.org.uk/mailman/listinfo/linux-arm-kernel FAQ: http://www.arm.linux.org.uk/mailinglists/faq.php Etiquette: http://www.arm.linux.org.uk/mailinglists/etiquette.php
Re: [patch 4/4] MFD: Change mfd platform device usage to wrapper platform_device
In reply to Ben DooksQuoted 5 lines · Ben Dooks
So, the enable/disable calls might be useful, however is there any reason this could not be handled by the clock framework? The suspend/resume entries where not used, and I belive should not be in here.
Hi Ben,
I agree with you on wrapping the platform device, however the enable / disable methods must stay.
Other cores need them, and they arent only there for clock support.
I've just looked at the TMIO MMC driver to refersh my memory and the suspend / resume hooks must stay also. TMIO MMC needs to be handled differently when suspending and when being disabled (loss of state when disabled).
As yet, only tc6393 is 'in tree' but Im waiting for the core to settle down before I convert my other TMIO based code (again again) to the new core.
So, in summary -
ACK to your changes to wrap the device as an 'mfd_device' NAK to deleting the mfd_cell power management.
Re: [patch 4/4] MFD: Change mfd platform device usage to wrapper platform_device
Quoted 11 lines · ian
Quoted 5 lines · Ben Dooks
So, the enable/disable calls might be useful, however is there any reason this could not be handled by the clock framework? The suspend/resume entries where not used, and I belive should not be in here.
Hi Ben,
I agree with you on wrapping the platform device, however the enable / disable methods must stay.
Ack on that. Also look also at the ds1wm driver. Right now it provides its own enable/disable methods in platform data because it predates the MFD core. It should be changed into an MFD cell driver, so enable/disable methods in mfd_cell would be most useful for it.
Quoted 13 lines
Other cores need them, and they arent only there for clock support.
I've just looked at the TMIO MMC driver to refersh my memory and the suspend / resume hooks must stay also. TMIO MMC needs to be handled differently when suspending and when being disabled (loss of state when disabled).
As yet, only tc6393 is 'in tree' but Im waiting for the core to settle down before I convert my other TMIO based code (again again) to the new core.
So, in summary -
Quoted 8 lines
ACK to your changes to wrap the device as an '
mfd_device' NAK to deleting themfd_cellpower management.--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
regards
Philipp
Re: [patch 4/4] MFD: Change mfd platform device usage to wrapper platform_device
In reply to DmitryQuoted 3 lines · Dmitry
NAK.
0) It was discussed yesterday on the list and the decision was to go in a different way.
It was?
I prefer the wrapped way personally...
Re: [patch 4/4] MFD: Change mfd platform device usage to wrapper platform_device
2008/7/9 ian <[email protected]>:
Quoted 8 lines · Dmitry
Quoted 3 lines
NAK.
0) It was discussed yesterday on the list and the decision was to go in a different way.It was?
I prefer the wrapped way personally...
In any case IMO it's better to call platform_device_register() rather than
device_initialise()/platform_device_add().
Samuel? Russell?
Signature · 2 lines
With best wishes Dmitry
Re: [patch 4/4] MFD: Change mfd platform device usage to wrapper platform_device
Quoted 6 lines · Dmitry
2008/7/9 ian <[email protected]>:
Quoted 5 lines · Dmitry
Quoted 3 lines
NAK.
0) It was discussed yesterday on the list and the decision was to go in a different way.
Quoted 8 lines
Quoted 3 lines
It was?
I prefer the wrapped way personally...
In any case IMO it's better to call
platform_device_register()rather than
device_initialise()/platform_device_add().Samuel? Russell?
WTF??? That's just completely wrong - assuming the internals of how the platform device alloc API works...
What it's clear from my *brief* read of this thread is that the MFD support doesn't seem to be ready for mainline yet - there's clearly issues here that need further work.
Given that, and where we are (there's maybe two of *my* days left until the merge window opens) I'm *very* tempted to drop the MFD support out of my tree for this merge window - which basically means removing 5127/1, 5128/1 and 5129/1.
Re: [patch 4/4] MFD: Change mfd platform device usage to wrapper platform_device
Quoted 19 lines · Russell King - ARM Linux
Quoted 6 lines · Dmitry
2008/7/9 ian <[email protected]>:
Quoted 5 lines · Dmitry
Quoted 3 lines
NAK.
0) It was discussed yesterday on the list and the decision was to go in a different way.Quoted 8 lines
Quoted 3 lines
It was?
I prefer the wrapped way personally...
In any case IMO it's better to call
platform_device_register()rather than
device_initialise()/platform_device_add().Samuel? Russell?
WTF??? That's just completely wrong - assuming the internals of how the platform device alloc API works...
Quoted 8 lines
What it's clear from my *brief* read of this thread is that the MFD support doesn't seem to be ready for mainline yet - there's clearly issues here that need further work.
Given that, and where we are (there's maybe two of *my* days left until the merge window opens) I'm *very* tempted to drop the MFD support out of my tree for this merge window - which basically means removing 5127/1, 5128/1 and 5129/1.
Why? We were talking about improvements, not the current code found in the referenced patches. They may be not the best ones, but they contain pretty clean code which works. The only issue with current code is that tc6393xb depends on some arm-specific defines, so we should either apply patch submitted today ("tc6393xb irq fixes") or make "depends on ARM".
Signature · 2 lines
With best wishes Dmitry
Re: [patch 4/4] MFD: Change mfd platform device usage to wrapper platform_device
In reply to Russell King - ARM LinuxQuoted 5 lines · Russell King - ARM Linux
Given that, and where we are (there's maybe two of *my* days left until the merge window opens) I'm *very* tempted to drop the MFD support out of my tree for this merge window - which basically means removing 5127/1, 5128/1 and 5129/1.
Please dont.
The code thats in already is perfectly ok for what its doing.
The discussion now is on ways to improve MFD for new devices, and somewhat tangential. All the improvements can wait until after 2.6.26.
Re: [patch 4/4] MFD: Change mfd platform device usage to wrapper platform_device
In reply to Russell King - ARM LinuxQuoted 14 lines · Russell King - ARM Linux
Quoted 13 lines · Dmitry
2008/7/9 ian <[email protected]>:
Quoted 8 lines · Dmitry
Quoted 3 lines
NAK.
0) It was discussed yesterday on the list and the decision was to go in a different way.It was?
I prefer the wrapped way personally...
In any case IMO it's better to call
platform_device_register()rather than
device_initialise()/platform_device_add().
Quoted 8 lines
Quoted 1 line
Samuel? Russell?
WTF??? That's just completely wrong - assuming the internals of how the platform device alloc API works...
What it's clear from my *brief* read of this thread is that the MFD support doesn't seem to be ready for mainline yet - there's clearly issues here that need further work.
I think the main issue at the moment is that it's not passing generic platform data if we ever want to use an existing driver as a cell driver. I'd say this is more of a lack of feature rather than broken code, so please dont drop the mfd-core patch.
Cheers,
Samuel.
Quoted 9 lines
Given that, and where we are (there's maybe two of *my* days left until the merge window opens) I'm *very* tempted to drop the MFD support out of my tree for this merge window - which basically means removing 5127/1, 5128/1 and 5129/1.
-------------------------------------------------------------------
List admin: http://lists.arm.linux.org.uk/mailman/listinfo/linux-arm-kernel FAQ: http://www.arm.linux.org.uk/mailinglists/faq.php Etiquette: http://www.arm.linux.org.uk/mailinglists/etiquette.php
Re: [patch 4/4] MFD: Change mfd platform device usage to wrapper platform_device
In reply to Ben DooksQuoted 3 lines · Ben Dooks
This patch changes the mfd core behaviour to wrapper the
platform_deviceit creates in an structmfd_devicewhich contains the information about the cell that was created.
You can't do this. Grab a reference to the platform device (by holding one of its sysfs files open) and then remove all the users of the mfd-core module and the mfd-core module itself.
Then, read from that file and close it. Watch your kernel oops.
That's why device release methods inside modules are a BAD IDEA and why we have the platform device alloc API.
Re: [patch 4/4] MFD: Change mfd platform device usage to wrapper platform_device
Quoted 5 lines · Russell King - ARM Linux
Quoted 3 lines · Ben Dooks
This patch changes the mfd core behaviour to wrapper the
platform_deviceit creates in an structmfd_devicewhich contains the information about the cell that was created.
Quoted 8 lines
You can't do this. Grab a reference to the platform device (by holding one of its sysfs files open) and then remove all the users of the mfd-core module and the mfd-core module itself.
Then, read from that file and close it. Watch your kernel oops.
That's why device release methods inside modules are a BAD IDEA and why we have the platform device alloc API.
Would this be fixed by grabbing a reference to the mfd-core module whilst the mfd device is open?
Signature · 4 lines
Ben Q: What's a light-year? A: One-third less calories than a regular year.