> -----Original Message-----
> From: Kevin Hilman [mailto:[email protected]]
> Sent: Tuesday, May 11, 2010 8:27 PM
> To: G, Manjunath Kondaiah
> Cc: [email protected]; Agarwal, Preshit; Tony
> Lindgren; Turquette, Mike; V, Hemanth
> Subject: Re: [PATCH v3] OMAP3: Registering sgx device and
> it's platform data
>
> Manjunatha GK <[email protected]> writes:
>
> > The SGX powervr_device is registered with it's platform specific
> > data to provide information about setting constraint through
> > omap_pm_set_min_bus_tput.
> >
> > Signed-off-by: Preshit Agarwal <[email protected]>
> > Signed-off-by: Manjunatha GK <[email protected]>
> > Cc: Tony Lindgren <[email protected]>
> > Cc: Kevin Hilman <[email protected]>
> > Cc: Mike Turquette <[email protected]>
> > Cc: Hemanth V <[email protected]>
> > ---
> > arch/arm/mach-omap2/devices.c | 26
> ++++++++++++++++++++++-
> > arch/arm/mach-omap2/include/mach/omap_sgxdef.h | 11 ++++++++++
> > 2 files changed, 35 insertions(+), 2 deletions(-)
> > create mode 100644 arch/arm/mach-omap2/include/mach/omap_sgxdef.h
> >
> > diff --git a/arch/arm/mach-omap2/devices.c
> b/arch/arm/mach-omap2/devices.c
> > index 20fa76e..aabbf7b 100644
> > --- a/arch/arm/mach-omap2/devices.c
> > +++ b/arch/arm/mach-omap2/devices.c
> > @@ -26,7 +26,7 @@
> > #include <plat/mux.h>
> > #include <mach/gpio.h>
> > #include <plat/mmc.h>
> > -
> > +#include <mach/omap_sgxdef.h>
> > #include "mux.h"
> >
> > #if defined(CONFIG_VIDEO_OMAP2) ||
> defined(CONFIG_VIDEO_OMAP2_MODULE)
> > @@ -791,6 +791,28 @@ static inline void omap_hdq_init(void)
> > static inline void omap_hdq_init(void) {}
> > #endif
> >
> > +struct sgx_platform_data omap_sgx_data = {
> > +#ifdef CONFIG_PM
> > + .set_min_bus_tput = omap_pm_set_min_bus_tput,
> > +#else
> > + .set_min_bus_tput = NULL,
> > +#endif
>
> Please explain why this should be dependent on CONFIG_PM?
>
> I suspect you should be making these kinds of decisions in the driver
> itself, not in device init.
Not sure in which context the SGX driver uses this platform data.
But, as you mentioned, we can leave it to SGX driver for handling it in
driver itself. I will make this as NULL in device init.
>
> > +};
> > +
> > +static struct platform_device powervr_device = {
> > + .name = "pvrsrvkm",
> > + .id = -1,
> > + .dev = {
> > + .platform_data = &omap_sgx_data,
> > + }
> > +};
> > +
> > +static void omap_init_sgx(void)
> > +{
> > + (void) platform_device_register(&powervr_device);
> > +}
> > +
> > +
> >
> /*------------------------------------------------------------
> -------------*/
> >
> > static int __init omap2_init_devices(void)
> > @@ -805,7 +827,7 @@ static int __init omap2_init_devices(void)
> > omap_hdq_init();
> > omap_init_sti();
> > omap_init_sha1_md5();
> > -
> > + omap_init_sgx();
> > return 0;
> > }
> > arch_initcall(omap2_init_devices);
> > diff --git a/arch/arm/mach-omap2/include/mach/omap_sgxdef.h
> b/arch/arm/mach-omap2/include/mach/omap_sgxdef.h
> > new file mode 100644
> > index 0000000..5d90a6a
> > --- /dev/null
> > +++ b/arch/arm/mach-omap2/include/mach/omap_sgxdef.h
> > @@ -0,0 +1,11 @@
> > +#ifndef OMAP_SGXDEF_H
> > +#define OMAP_SGXDEF_H
> > +
> > +#include <plat/omap-pm.h>
> > +
> > +struct sgx_platform_data {
> > + void(*set_min_bus_tput)(struct device *dev, u8 agent_id,
>
> need a space after 'void' and before '('
Ok.
>
> > +
> unsigned long r);
>
> minor nit: you could drop a tab of 3 for nicer looking alignment
Ok. Thanks
-Manjunath
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html
