Patchwork fsmc-nand: Add fsmc_nand_set_plat_data in linux/mtd/fsmc.h

login
register
mail settings
Submitter Viresh KUMAR
Date Feb. 23, 2011, 6:49 a.m.
Message ID <e63303f4cd51aa61f08e2b026b6a80dc0d9cd54a.1298442421.git.viresh.kumar@st.com>
Download mbox | patch
Permalink /patch/84107/
State New
Headers show

Comments

Viresh KUMAR - Feb. 23, 2011, 6:49 a.m.
In most of the cases partitions info, width, etc comes from board files. And
device structure may be defined in machine files, common to all board files.
Thus, we need to set platform data from board file, for which
fsmc_nand_set_plat_data routine is required.

Signed-off-by: Viresh Kumar <viresh.kumar@st.com>
Signed-off-by: Vipin Kumar <vipin.kumar@st.com>
---
 include/linux/mtd/fsmc.h |   17 +++++++++++++++++
 1 files changed, 17 insertions(+), 0 deletions(-)
Linus Walleij - Feb. 24, 2011, 12:38 p.m.
2011/2/23 Viresh Kumar <viresh.kumar@st.com>:

> In most of the cases partitions info, width, etc comes from board files. And
> device structure may be defined in machine files, common to all board files.
> Thus, we need to set platform data from board file, for which
> fsmc_nand_set_plat_data routine is required.

Hm you create a new function, but where is it used? If it's used by a later
patch perhaps you should make a patch series instead?

Yours,
Linus Walleij
Viresh KUMAR - Feb. 25, 2011, 3:44 a.m.
On 02/24/2011 06:08 PM, Linus Walleij wrote:
> 2011/2/23 Viresh Kumar <viresh.kumar@st.com>:
> 
>> > In most of the cases partitions info, width, etc comes from board files. And
>> > device structure may be defined in machine files, common to all board files.
>> > Thus, we need to set platform data from board file, for which
>> > fsmc_nand_set_plat_data routine is required.
> Hm you create a new function, but where is it used? If it's used by a later
> patch perhaps you should make a patch series instead?

Linus,

Actually it will be used in SPEAr patches, which will be sent separately. I could
have kept it as part of that series also, but just thought to send it separately.
You want me to keep it in spear patch series instead?
Linus Walleij - Feb. 25, 2011, 8:39 a.m.
2011/2/25 viresh kumar <viresh.kumar@st.com>:
> On 02/24/2011 06:08 PM, Linus Walleij wrote:
>> 2011/2/23 Viresh Kumar <viresh.kumar@st.com>:
>>
>>> > In most of the cases partitions info, width, etc comes from board files. And
>>> > device structure may be defined in machine files, common to all board files.
>>> > Thus, we need to set platform data from board file, for which
>>> > fsmc_nand_set_plat_data routine is required.
>> Hm you create a new function, but where is it used? If it's used by a later
>> patch perhaps you should make a patch series instead?
>
> Actually it will be used in SPEAr patches, which will be sent separately. I could
> have kept it as part of that series also, but just thought to send it separately.
> You want me to keep it in spear patch series instead?

Nah no problem I get it now...
Acked-by: Linus Walleij <linus.walleij@linaro.org>

Yours,
Linus Walleij
Artem Bityutskiy - Feb. 25, 2011, 11:07 a.m.
On Fri, 2011-02-25 at 09:14 +0530, viresh kumar wrote:
> On 02/24/2011 06:08 PM, Linus Walleij wrote:
> > 2011/2/23 Viresh Kumar <viresh.kumar@st.com>:
> > 
> >> > In most of the cases partitions info, width, etc comes from board files. And
> >> > device structure may be defined in machine files, common to all board files.
> >> > Thus, we need to set platform data from board file, for which
> >> > fsmc_nand_set_plat_data routine is required.
> > Hm you create a new function, but where is it used? If it's used by a later
> > patch perhaps you should make a patch series instead?
> 
> Linus,
> 
> Actually it will be used in SPEAr patches, which will be sent separately. I could
> have kept it as part of that series also, but just thought to send it separately.
> You want me to keep it in spear patch series instead?

I think this is important piece of information which should be in the
patch commit message.
Artem Bityutskiy - Feb. 25, 2011, 11:11 a.m.
On Wed, 2011-02-23 at 12:19 +0530, Viresh Kumar wrote:
> In most of the cases partitions info, width, etc comes from board files. And
> device structure may be defined in machine files, common to all board files.
> Thus, we need to set platform data from board file, for which
> fsmc_nand_set_plat_data routine is required.
> 
> Signed-off-by: Viresh Kumar <viresh.kumar@st.com>
> Signed-off-by: Vipin Kumar <vipin.kumar@st.com>
> ---
>  include/linux/mtd/fsmc.h |   17 +++++++++++++++++
>  1 files changed, 17 insertions(+), 0 deletions(-)
> 
> diff --git a/include/linux/mtd/fsmc.h b/include/linux/mtd/fsmc.h
> index 6987995..0cba324 100644
> --- a/include/linux/mtd/fsmc.h
> +++ b/include/linux/mtd/fsmc.h
> @@ -160,4 +160,21 @@ extern void __init fsmc_init_board_info(struct platform_device *pdev,
>  		struct mtd_partition *partitions, unsigned int nr_partitions,
>  		unsigned int width);
>  
> +/* This function is used to set platform data field of pdev->dev */
> +static inline void fsmc_nand_set_plat_data(struct platform_device *pdev,
> +		struct mtd_partition *partitions, unsigned int nr_partitions,
> +		unsigned int options, unsigned int width)
> +{
> +	struct fsmc_nand_platform_data *plat_data;
> +	plat_data = dev_get_platdata(&pdev->dev);
> +
> +	if (partitions) {
> +		plat_data->partitions = partitions;
> +		plat_data->nr_partitions = nr_partitions;
> +	}
> +
> +	plat_data->options = options;
> +	plat_data->width = width;
> +}

Isn't this function too big to be an inline func in a header file?
Viresh KUMAR - Feb. 25, 2011, 11:11 a.m.
On 02/25/2011 04:37 PM, Artem Bityutskiy wrote:
> On Fri, 2011-02-25 at 09:14 +0530, viresh kumar wrote:
>> > On 02/24/2011 06:08 PM, Linus Walleij wrote:
>>> > > 2011/2/23 Viresh Kumar <viresh.kumar@st.com>:
>>> > > 
>>>>> > >> > In most of the cases partitions info, width, etc comes from board files. And
>>>>> > >> > device structure may be defined in machine files, common to all board files.
>>>>> > >> > Thus, we need to set platform data from board file, for which
>>>>> > >> > fsmc_nand_set_plat_data routine is required.
>>> > > Hm you create a new function, but where is it used? If it's used by a later
>>> > > patch perhaps you should make a patch series instead?
>> > 
>> > Linus,
>> > 
>> > Actually it will be used in SPEAr patches, which will be sent separately. I could
>> > have kept it as part of that series also, but just thought to send it separately.
>> > You want me to keep it in spear patch series instead?
> I think this is important piece of information which should be in the
> patch commit message.

Ok. I will resend it.
Artem Bityutskiy - Feb. 25, 2011, 11:12 a.m.
On Fri, 2011-02-25 at 16:41 +0530, viresh kumar wrote:
> On 02/25/2011 04:37 PM, Artem Bityutskiy wrote:
> > On Fri, 2011-02-25 at 09:14 +0530, viresh kumar wrote:
> >> > On 02/24/2011 06:08 PM, Linus Walleij wrote:
> >>> > > 2011/2/23 Viresh Kumar <viresh.kumar@st.com>:
> >>> > > 
> >>>>> > >> > In most of the cases partitions info, width, etc comes from board files. And
> >>>>> > >> > device structure may be defined in machine files, common to all board files.
> >>>>> > >> > Thus, we need to set platform data from board file, for which
> >>>>> > >> > fsmc_nand_set_plat_data routine is required.
> >>> > > Hm you create a new function, but where is it used? If it's used by a later
> >>> > > patch perhaps you should make a patch series instead?
> >> > 
> >> > Linus,
> >> > 
> >> > Actually it will be used in SPEAr patches, which will be sent separately. I could
> >> > have kept it as part of that series also, but just thought to send it separately.
> >> > You want me to keep it in spear patch series instead?
> > I think this is important piece of information which should be in the
> > patch commit message.
> 
> Ok. I will resend it.

To elaborate, imagine I put this patch to my l2 tree, then David has the
same question when he decides whether to pick it or not.
Viresh KUMAR - Feb. 25, 2011, 11:22 a.m.
On 02/25/2011 04:41 PM, Artem Bityutskiy wrote:
>> > diff --git a/include/linux/mtd/fsmc.h b/include/linux/mtd/fsmc.h
>> > index 6987995..0cba324 100644
>> > --- a/include/linux/mtd/fsmc.h
>> > +++ b/include/linux/mtd/fsmc.h
>> > @@ -160,4 +160,21 @@ extern void __init fsmc_init_board_info(struct platform_device *pdev,
>> >  		struct mtd_partition *partitions, unsigned int nr_partitions,
>> >  		unsigned int width);
>> >  
>> > +/* This function is used to set platform data field of pdev->dev */
>> > +static inline void fsmc_nand_set_plat_data(struct platform_device *pdev,
>> > +		struct mtd_partition *partitions, unsigned int nr_partitions,
>> > +		unsigned int options, unsigned int width)
>> > +{
>> > +	struct fsmc_nand_platform_data *plat_data;
>> > +	plat_data = dev_get_platdata(&pdev->dev);
>> > +
>> > +	if (partitions) {
>> > +		plat_data->partitions = partitions;
>> > +		plat_data->nr_partitions = nr_partitions;
>> > +	}
>> > +
>> > +	plat_data->options = options;
>> > +	plat_data->width = width;
>> > +}
> Isn't this function too big to be an inline func in a header file?
> 

Sorry, i have already resent this patch with change in commit message.

Actually this routine will not be called many times. Only for setting
plat_data of few devices (depending on board). So i thought keeping
it inline would be better. But will change if you want. Should i move
it in fsmc-nand.c??
Artem Bityutskiy - Feb. 25, 2011, 11:25 a.m.
On Fri, 2011-02-25 at 16:52 +0530, viresh kumar wrote:
> Actually this routine will not be called many times.

Which means it is not on the hot path which means inline is not really
needed. Also, today it is not called too many times, tomorrow it could
be called more often...

>  Only for setting
> plat_data of few devices (depending on board). So i thought keeping
> it inline would be better. But will change if you want. Should i move
> it in fsmc-nand.c??

Hi, up to you, I would not keep it in a header file, this is more about
personal taste, I guess.
Viresh KUMAR - Feb. 25, 2011, 11:43 a.m.
On 02/25/2011 04:55 PM, Artem Bityutskiy wrote:
> On Fri, 2011-02-25 at 16:52 +0530, viresh kumar wrote:
>> > Actually this routine will not be called many times.
> Which means it is not on the hot path which means inline is not really
> needed. Also, today it is not called too many times, tomorrow it could
> be called more often...
> 
>> >  Only for setting
>> > plat_data of few devices (depending on board). So i thought keeping
>> > it inline would be better. But will change if you want. Should i move
>> > it in fsmc-nand.c??
> Hi, up to you, I would not keep it in a header file, this is more about
> personal taste, I guess.

Ok. I have moved this to fsmc-nand.c. Will send V2.

Patch

diff --git a/include/linux/mtd/fsmc.h b/include/linux/mtd/fsmc.h
index 6987995..0cba324 100644
--- a/include/linux/mtd/fsmc.h
+++ b/include/linux/mtd/fsmc.h
@@ -160,4 +160,21 @@  extern void __init fsmc_init_board_info(struct platform_device *pdev,
 		struct mtd_partition *partitions, unsigned int nr_partitions,
 		unsigned int width);
 
+/* This function is used to set platform data field of pdev->dev */
+static inline void fsmc_nand_set_plat_data(struct platform_device *pdev,
+		struct mtd_partition *partitions, unsigned int nr_partitions,
+		unsigned int options, unsigned int width)
+{
+	struct fsmc_nand_platform_data *plat_data;
+	plat_data = dev_get_platdata(&pdev->dev);
+
+	if (partitions) {
+		plat_data->partitions = partitions;
+		plat_data->nr_partitions = nr_partitions;
+	}
+
+	plat_data->options = options;
+	plat_data->width = width;
+}
+
 #endif /* __MTD_FSMC_H */