On Sat, Oct 16, 2021 at 12:27:33PM +0530, Manivannan Sadhasivam wrote:
> From: Loic Poulain <[email protected]>
>
> Currently, the MHI controller driver defines which channels should
> have their inbound buffers allocated and queued. But ideally, this is
> something that should be decided by the MHI device driver instead,
> which actually deals with that buffers.
>
> Add a flag parameter to mhi_prepare_for_transfer allowing to specify
> if buffers have to be allocated and queued by the MHI stack.
This is a horrible api. Now one has to go and look up why "0" was added
to a function as a parameter.
If you don't want to allocate the buffer, then make a function of that
name and call that. As you only have one "flag", don't try to make
something generic here that is obviously not generic at all.
You all can do better than this.
thanks,
greg k-h
On Sat, Oct 16, 2021 at 09:39:53AM +0200, Greg KH wrote:
> On Sat, Oct 16, 2021 at 12:27:33PM +0530, Manivannan Sadhasivam wrote:
> > From: Loic Poulain <[email protected]>
> >
> > Currently, the MHI controller driver defines which channels should
> > have their inbound buffers allocated and queued. But ideally, this is
> > something that should be decided by the MHI device driver instead,
> > which actually deals with that buffers.
> >
> > Add a flag parameter to mhi_prepare_for_transfer allowing to specify
> > if buffers have to be allocated and queued by the MHI stack.
>
> This is a horrible api. Now one has to go and look up why "0" was added
> to a function as a parameter.
>
> If you don't want to allocate the buffer, then make a function of that
> name and call that. As you only have one "flag", don't try to make
> something generic here that is obviously not generic at all.
>
This is the only API that can be used by the client drivers to pass the
configurations to the MHI stack. So we wanted to have a flags parameter that
could be extended in the future also.
Regarding "0", the default behaviour is to not pre allocate the buffer at all.
So it made less sense to add a separate flag or an API for that.
Thanks,
Mani
> You all can do better than this.
>
> thanks,
>
> greg k-h
On Sat, Oct 16, 2021 at 10:01:28PM +0530, Manivannan Sadhasivam wrote:
> On Sat, Oct 16, 2021 at 09:39:53AM +0200, Greg KH wrote:
> > On Sat, Oct 16, 2021 at 12:27:33PM +0530, Manivannan Sadhasivam wrote:
> > > From: Loic Poulain <[email protected]>
> > >
> > > Currently, the MHI controller driver defines which channels should
> > > have their inbound buffers allocated and queued. But ideally, this is
> > > something that should be decided by the MHI device driver instead,
> > > which actually deals with that buffers.
> > >
> > > Add a flag parameter to mhi_prepare_for_transfer allowing to specify
> > > if buffers have to be allocated and queued by the MHI stack.
> >
> > This is a horrible api. Now one has to go and look up why "0" was added
> > to a function as a parameter.
> >
> > If you don't want to allocate the buffer, then make a function of that
> > name and call that. As you only have one "flag", don't try to make
> > something generic here that is obviously not generic at all.
> >
>
> This is the only API that can be used by the client drivers to pass the
> configurations to the MHI stack. So we wanted to have a flags parameter that
> could be extended in the future also.
Worry about future issues then, in the future :)
> Regarding "0", the default behaviour is to not pre allocate the buffer at all.
> So it made less sense to add a separate flag or an API for that.
But again, this is now hard to understand and if you run across a '0' in
the call, you have to go and look it up, breaking your reading flow.
Please just create a new function for this new option, and then have
both of them call the common function with a boolean for this "allocate
or not" type of thing. We do this all the time in the kernel to make it
easier to read and understand over time.
thanks,
greg k-h