Re: [PATCH v4 01/10] mhi: Add mhi_controller_initialize helper

[Hemant Kumar <hemantk@codeaurora.org>]

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