| Message ID | e63303f4cd51aa61f08e2b026b6a80dc0d9cd54a.1298442421.git.viresh.kumar@st.com |
|---|---|
| State | New, archived |
| Headers | show |
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
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?
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
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.
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?
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.
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.
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??
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.
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.
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 */