| Message ID | 1607594575-31590-2-git-send-email-loic.poulain@linaro.org |
|---|---|
| State | Superseded |
| Headers | show |
| Series | mhi: pci_generic: Misc improvements | expand |
On 12/10/20 2:02 AM, Loic Poulain wrote: > This function allows to initialize a mhi_controller structure. > Today, it only zeroing the structure. > > Use this function from mhi_alloc_controller so that any further > initialization can be factorized in initalize function. > > Signed-off-by: Loic Poulain <loic.poulain@linaro.org> > --- > drivers/bus/mhi/core/init.c | 7 +++++++ > include/linux/mhi.h | 6 ++++++ > 2 files changed, 13 insertions(+) > > diff --git a/drivers/bus/mhi/core/init.c b/drivers/bus/mhi/core/init.c > index 96cde9c..4acad28 100644 > --- a/drivers/bus/mhi/core/init.c > +++ b/drivers/bus/mhi/core/init.c > @@ -1021,11 +1021,18 @@ void mhi_unregister_controller(struct mhi_controller *mhi_cntrl) > } > EXPORT_SYMBOL_GPL(mhi_unregister_controller); > > +void mhi_initialize_controller(struct mhi_controller *mhi_cntrl) > +{ > + memset(mhi_cntrl, 0, sizeof(*mhi_cntrl)); > +} > +EXPORT_SYMBOL_GPL(mhi_initialize_controller); > + > struct mhi_controller *mhi_alloc_controller(void) > { > struct mhi_controller *mhi_cntrl; > > mhi_cntrl = kzalloc(sizeof(*mhi_cntrl), GFP_KERNEL); > + mhi_initialize_controller(mhi_cntrl); Considering the kzalloc, do we really need to call here as well ? > > return mhi_cntrl; > } > diff --git a/include/linux/mhi.h b/include/linux/mhi.h > index 04cf7f3..2754742 100644 > --- a/include/linux/mhi.h > +++ b/include/linux/mhi.h > @@ -537,6 +537,12 @@ struct mhi_driver { > #define to_mhi_device(dev) container_of(dev, struct mhi_device, dev) > > /** > + * mhi_initialize_controller - Initialize MHI Controller structure > + * @mhi_cntrl: MHI controller structure to initialize > + */ > +void mhi_initialize_controller(struct mhi_controller *mhi_cntrl); > + > +/** > * mhi_alloc_controller - Allocate the MHI Controller structure > * Allocate the mhi_controller structure using zero initialized memory > */ > Thanks, Hemant -- The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum, a Linux Foundation Collaborative Project
Hi Loic, On 12/11/20 11:13 AM, Loic Poulain wrote: > Hi Hemant, > > Le ven. 11 déc. 2020 à 19:57, Hemant Kumar <hemantk@codeaurora.org > <mailto:hemantk@codeaurora.org>> a écrit : > > > > On 12/10/20 2:02 AM, Loic Poulain wrote: > > This function allows to initialize a mhi_controller structure. > > Today, it only zeroing the structure. > > > > Use this function from mhi_alloc_controller so that any further > > initialization can be factorized in initalize function. > > > > Signed-off-by: Loic Poulain <loic.poulain@linaro.org > <mailto:loic.poulain@linaro.org>> > > --- > > drivers/bus/mhi/core/init.c | 7 +++++++ > > include/linux/mhi.h | 6 ++++++ > > 2 files changed, 13 insertions(+) > > > > diff --git a/drivers/bus/mhi/core/init.c > b/drivers/bus/mhi/core/init.c > > index 96cde9c..4acad28 100644 > > --- a/drivers/bus/mhi/core/init.c > > +++ b/drivers/bus/mhi/core/init.c > > @@ -1021,11 +1021,18 @@ void mhi_unregister_controller(struct > mhi_controller *mhi_cntrl) > > } > > EXPORT_SYMBOL_GPL(mhi_unregister_controller); > > > > +void mhi_initialize_controller(struct mhi_controller *mhi_cntrl) > > +{ > > + memset(mhi_cntrl, 0, sizeof(*mhi_cntrl)); > > +} > > +EXPORT_SYMBOL_GPL(mhi_initialize_controller); > > + > > struct mhi_controller *mhi_alloc_controller(void) > > { > > struct mhi_controller *mhi_cntrl; > > > > mhi_cntrl = kzalloc(sizeof(*mhi_cntrl), GFP_KERNEL); > > + mhi_initialize_controller(mhi_cntrl); > Considering the kzalloc, do we really need to call here as well ? > > > To summary back and forth on that change, the point is that mhi_cntrl > may have some extra initialization in the futur (e.g mutex init...) and > so a helper is needed to correctly initialize the structure, though > today it does nothing except zeroing. I am aware of the discussion and reason for introducing new exported API. Based on my understanding, as of now you are going to call the new exported API in MHI controller driver. I was thinking of adding new API in mhi_alloc_controller only after introducing extra initialization in future, without that it was looking redundant to me at this point of time. > > Regards, > Loïc > > > > > > return mhi_cntrl; > > } > > diff --git a/include/linux/mhi.h b/include/linux/mhi.h > > index 04cf7f3..2754742 100644 > > --- a/include/linux/mhi.h > > +++ b/include/linux/mhi.h > > @@ -537,6 +537,12 @@ struct mhi_driver { > > #define to_mhi_device(dev) container_of(dev, struct mhi_device, > dev) > > > > /** > > + * mhi_initialize_controller - Initialize MHI Controller structure > > + * @mhi_cntrl: MHI controller structure to initialize > > + */ > > +void mhi_initialize_controller(struct mhi_controller *mhi_cntrl); > > + > > +/** > > * mhi_alloc_controller - Allocate the MHI Controller structure > > * Allocate the mhi_controller structure using zero initialized > memory > > */ > > > > Thanks, > Hemant > -- > The Qualcomm Innovation Center, Inc. is a member of the Code Aurora > Forum, > a Linux Foundation Collaborative Project > Thanks, Hemant -- The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum, a Linux Foundation Collaborative Project
On Fri, 11 Dec 2020 at 21:01, Hemant Kumar <hemantk@codeaurora.org> wrote: > > Hi Loic, > > On 12/11/20 11:13 AM, Loic Poulain wrote: > > Hi Hemant, > > > > Le ven. 11 déc. 2020 à 19:57, Hemant Kumar <hemantk@codeaurora.org > > <mailto:hemantk@codeaurora.org>> a écrit : > > > > > > > > On 12/10/20 2:02 AM, Loic Poulain wrote: > > > This function allows to initialize a mhi_controller structure. > > > Today, it only zeroing the structure. > > > > > > Use this function from mhi_alloc_controller so that any further > > > initialization can be factorized in initalize function. > > > > > > Signed-off-by: Loic Poulain <loic.poulain@linaro.org > > <mailto:loic.poulain@linaro.org>> > > > --- > > > drivers/bus/mhi/core/init.c | 7 +++++++ > > > include/linux/mhi.h | 6 ++++++ > > > 2 files changed, 13 insertions(+) > > > > > > diff --git a/drivers/bus/mhi/core/init.c > > b/drivers/bus/mhi/core/init.c > > > index 96cde9c..4acad28 100644 > > > --- a/drivers/bus/mhi/core/init.c > > > +++ b/drivers/bus/mhi/core/init.c > > > @@ -1021,11 +1021,18 @@ void mhi_unregister_controller(struct > > mhi_controller *mhi_cntrl) > > > } > > > EXPORT_SYMBOL_GPL(mhi_unregister_controller); > > > > > > +void mhi_initialize_controller(struct mhi_controller *mhi_cntrl) > > > +{ > > > + memset(mhi_cntrl, 0, sizeof(*mhi_cntrl)); > > > +} > > > +EXPORT_SYMBOL_GPL(mhi_initialize_controller); > > > + > > > struct mhi_controller *mhi_alloc_controller(void) > > > { > > > struct mhi_controller *mhi_cntrl; > > > > > > mhi_cntrl = kzalloc(sizeof(*mhi_cntrl), GFP_KERNEL); > > > + mhi_initialize_controller(mhi_cntrl); > > Considering the kzalloc, do we really need to call here as well ? > > > > > > To summary back and forth on that change, the point is that mhi_cntrl > > may have some extra initialization in the futur (e.g mutex init...) and > > so a helper is needed to correctly initialize the structure, though > > today it does nothing except zeroing. > I am aware of the discussion and reason for introducing new exported > API. Based on my understanding, as of now you are going to call the new > exported API in MHI controller driver. I was thinking of adding new API > in mhi_alloc_controller only after introducing extra initialization in > future, without that it was looking redundant to me at this point of time. Ok, will remove that line. Loic
diff --git a/drivers/bus/mhi/core/init.c b/drivers/bus/mhi/core/init.c index 96cde9c..4acad28 100644 --- a/drivers/bus/mhi/core/init.c +++ b/drivers/bus/mhi/core/init.c @@ -1021,11 +1021,18 @@ void mhi_unregister_controller(struct mhi_controller *mhi_cntrl) } EXPORT_SYMBOL_GPL(mhi_unregister_controller); +void mhi_initialize_controller(struct mhi_controller *mhi_cntrl) +{ + memset(mhi_cntrl, 0, sizeof(*mhi_cntrl)); +} +EXPORT_SYMBOL_GPL(mhi_initialize_controller); + struct mhi_controller *mhi_alloc_controller(void) { struct mhi_controller *mhi_cntrl; mhi_cntrl = kzalloc(sizeof(*mhi_cntrl), GFP_KERNEL); + mhi_initialize_controller(mhi_cntrl); return mhi_cntrl; } diff --git a/include/linux/mhi.h b/include/linux/mhi.h index 04cf7f3..2754742 100644 --- a/include/linux/mhi.h +++ b/include/linux/mhi.h @@ -537,6 +537,12 @@ struct mhi_driver { #define to_mhi_device(dev) container_of(dev, struct mhi_device, dev) /** + * mhi_initialize_controller - Initialize MHI Controller structure + * @mhi_cntrl: MHI controller structure to initialize + */ +void mhi_initialize_controller(struct mhi_controller *mhi_cntrl); + +/** * mhi_alloc_controller - Allocate the MHI Controller structure * Allocate the mhi_controller structure using zero initialized memory */
This function allows to initialize a mhi_controller structure. Today, it only zeroing the structure. Use this function from mhi_alloc_controller so that any further initialization can be factorized in initalize function. Signed-off-by: Loic Poulain <loic.poulain@linaro.org> --- drivers/bus/mhi/core/init.c | 7 +++++++ include/linux/mhi.h | 6 ++++++ 2 files changed, 13 insertions(+) -- 2.7.4