Hi bbhatt:
On Wednesday, October 28, 2020 9:02 AM, bbhatt wrote:
> Hi Carl,
>
> Yes this change is needed. Good catch. I ran in to this issue as well when a
> dev_err() call was made with a bad MHI configuration.
[carl.yin] yes, I also meet this error with a bad MHI configuration.
>
> Maybe you can explain a little more in the commit text subject?
>
> You could say, "Fix null pointer access when parsing MHI configuration"?
>
> On 2020-10-26 22:33, [email protected] wrote:
> > From: carl <[email protected]>
> >
> > function parse_ev_cfg and parse_ch_cfg access mhi_cntrl->mhi_dev
> Functions parse_ev_cfg() and parse_ch_cfg()
> > before it is set in function mhi_register_controller,
> mhi_register_controller()
> > use cntrl_dev to instead mhi_dev.
> use cntrl_dev instead of mhi_dev.
> >
> > Signed-off-by: carl <[email protected]>
> With these commit text updates,
[carl.yin] thank for words correction, for my mother language is not English,
there are lots of words wrong in the commit.
>
> Reviewed-by: Bhaumik Bhatt <[email protected]>
> > ---
> > drivers/bus/mhi/core/init.c | 4 ++--
> > 1 file changed, 2 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/bus/mhi/core/init.c b/drivers/bus/mhi/core/init.c
> > index 0ffdebde8..c6b43e90b 100644
> > --- a/drivers/bus/mhi/core/init.c
> > +++ b/drivers/bus/mhi/core/init.c
> > @@ -610,7 +610,7 @@ static int parse_ev_cfg(struct mhi_controller
> > *mhi_cntrl, {
> > struct mhi_event *mhi_event;
> > const struct mhi_event_config *event_cfg;
> > - struct device *dev = &mhi_cntrl->mhi_dev->dev;
> > + struct device *dev = mhi_cntrl->cntrl_dev;
> > int i, num;
> >
> > num = config->num_events;
> > @@ -692,7 +692,7 @@ static int parse_ch_cfg(struct mhi_controller
> > *mhi_cntrl,
> > const struct mhi_controller_config *config) {
> > const struct mhi_channel_config *ch_cfg;
> > - struct device *dev = &mhi_cntrl->mhi_dev->dev;
> > + struct device *dev = mhi_cntrl->cntrl_dev;
> > int i;
> > u32 chan;
>
> --
> The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum, a
> Linux Foundation Collaborative Project
On 2020-10-27 19:03, Carl Yin wrote:
> Hi bbhatt:
>
> On Wednesday, October 28, 2020 9:02 AM, bbhatt wrote:
>
>> Hi Carl,
>>
>> Yes this change is needed. Good catch. I ran in to this issue as well
>> when a
>> dev_err() call was made with a bad MHI configuration.
> [carl.yin] yes, I also meet this error with a bad MHI configuration.
>>
>> Maybe you can explain a little more in the commit text subject?
>>
>> You could say, "Fix null pointer access when parsing MHI
>> configuration"?
>>
>> On 2020-10-26 22:33, [email protected] wrote:
>> > From: carl <[email protected]>
>> >
>> > function parse_ev_cfg and parse_ch_cfg access mhi_cntrl->mhi_dev
>> Functions parse_ev_cfg() and parse_ch_cfg()
>> > before it is set in function mhi_register_controller,
>> mhi_register_controller()
>> > use cntrl_dev to instead mhi_dev.
>> use cntrl_dev instead of mhi_dev.
>> >
>> > Signed-off-by: carl <[email protected]>
>> With these commit text updates,
> [carl.yin] thank for words correction, for my mother language is not
> English,
> there are lots of words wrong in the commit.
>>
No problem.
>> Reviewed-by: Bhaumik Bhatt <[email protected]>
>> > ---
>> > drivers/bus/mhi/core/init.c | 4 ++--
>> > 1 file changed, 2 insertions(+), 2 deletions(-)
>> >
>> > diff --git a/drivers/bus/mhi/core/init.c b/drivers/bus/mhi/core/init.c
>> > index 0ffdebde8..c6b43e90b 100644
>> > --- a/drivers/bus/mhi/core/init.c
>> > +++ b/drivers/bus/mhi/core/init.c
>> > @@ -610,7 +610,7 @@ static int parse_ev_cfg(struct mhi_controller
>> > *mhi_cntrl, {
>> > struct mhi_event *mhi_event;
>> > const struct mhi_event_config *event_cfg;
>> > - struct device *dev = &mhi_cntrl->mhi_dev->dev;
>> > + struct device *dev = mhi_cntrl->cntrl_dev;
>> > int i, num;
>> >
>> > num = config->num_events;
>> > @@ -692,7 +692,7 @@ static int parse_ch_cfg(struct mhi_controller
>> > *mhi_cntrl,
>> > const struct mhi_controller_config *config) {
>> > const struct mhi_channel_config *ch_cfg;
>> > - struct device *dev = &mhi_cntrl->mhi_dev->dev;
>> > + struct device *dev = mhi_cntrl->cntrl_dev;
>> > int i;
>> > u32 chan;
>>
>> --
>> The Qualcomm Innovation Center, Inc. is a member of the Code Aurora
>> Forum, a
>> Linux Foundation Collaborative Project
Can you also add a "Fixes:" tag to the patch?
Refer Documentation/process/submitting-patches.rst:
If your patch fixes a bug in a specific commit, e.g. you found an issue
using
``git bisect``, please use the 'Fixes:' tag with the first 12 characters
of
the SHA-1 ID, and the one line summary. Do not split the tag across
multiple
lines, tags are exempt from the "wrap at 75 columns" rule in order to
simplify
parsing scripts. For example::
Fixes: 54a4f0239f2e ("KVM: MMU: make kvm_mmu_zap_page() return the
number of pages it actually freed")
Thanks,
Bhaumik
--
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora
Forum,
a Linux Foundation Collaborative Project