Re: [PATCH v4] mfd: dm355evm_msp: Refactoring for add_child()

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



> But the change-log in this patch says "I did some stuff".
> What stuff did you change?  Which review comments did you
> tend to?

I imagine that I could increase the description granularity
to a detail level which you might also not like.


>>>> +put_device:
>>>> +	platform_device_put(pdev);
>>>> +	dev_err(&client->dev, "failed to add device %s\n", name);
>>>
>>> ... and remove this line.
>>
>> Do you really want that this error message should be deleted?
>>
>> How does this response fit to your request to introduce such a message
>> for the function "add_numbered_child" (on 2016-06-08)?
>> https://www.mail-archive.com/linux-kernel@xxxxxxxxxxxxxxx/msg1162299.html
>> https://lkml.org/lkml/2016/6/8/467
> 
> You've lost the context.

I interpreted the suggested message adjustments as separate changes.
So I wondered about a different handling for the Linux modules
"dm355evm_msp" and "twl-core".


> The "..." is meant to intimate that it
> follows on from a previous comment.  In this case:
> 
>>  > status = platform_device_add_data(pdev, pdata, pdata_len);
>>  > if (status < 0) {
>>  > 	dev_dbg(&pdev->dev, "can't add platform_data\n");
>>
>>  Please take the opportunity to convert these to dev_err()s.
> 
> So, convert the specific dev_dbg() calls to dev_err() and remove the
> contentless one at the bottom.

It seems then that you would like to get rid of an error message
at the end while increasing the importance of a related information.

Regards,
Markus
--
To unsubscribe from this list: send the line "unsubscribe kernel-janitors" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html



[Index of Archives]     [Kernel Development]     [Kernel Announce]     [Kernel Newbies]     [Linux Networking Development]     [Share Photos]     [IDE]     [Security]     [Git]     [Netfilter]     [Yosemite News]     [MIPS Linux]     [ARM Linux]     [Device Mapper]

  Powered by Linux