Re: [PATCH v5 1/3] USB: serial: option: remove old LARA-R6 PID

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

 



On Tue, Nov 15, 2022 at 05:46:54PM +0100, Davide Tronchin wrote:
> My apologies, this is my first attempt to submit a patch to the kernel
> community.
> 
> > We've asked you repeatedly whether you for u-blox or not, but you keep
> > ignoring this question. Knowing this would allow us to better evaluate
> > the reasoning and motivation behind this this change.
> 
> Yes, i am a u-blox employee and i've been asked to integrate LARA-L6 in
> the linux kernel and update the current code for LARA-R6 00B (updating
> the PID from 0x90fa to 0x908b).

Thanks. Was it it also a colleague of yours that submitted the initial
PID then?

> > The above commit message still does not explain why you want to remove
> > it and whether it would be safe to do so. Why was added in the first
> > place? What u-blox products used the old PID?
> 
> The first prototype of LARA-R6 00B had 0x90fa PID but, just before the
> product finalization, it has been decided to adopt a new USB composition
> and consequently a change of PID was necessary.
> The 0x90fa PID has been used only for some internal prototypes, hence
> no u-blox products with that PID have been shipped to customers.
> As pointed out in the discussion, the 0x90fa PID is used by other module
> vendors which sell Qualcomm based modems, hence i proposed to remove the
> association between u-blox (thedefine UBLOX_PRODUCT_R6XX) and 0x90fa,
> moving it directly in the option_ids array.

Thanks, this is the kind of details we've been asking for. Please put
some of this in the commit in some form so that it is obvious from just
reading the commit message that the patch is correct and safe to apply.

Make sure to mention that this Qualcomm PID is used by other products
and that's why you're leaving it in. Perhaps a Link tag with a
reference to Lars message pointing this out is in place. For example:

Link: https://lore.kernel.org/all/6572c4e6-d8bc-b8d3-4396-d879e4e76338@xxxxxxxxx/

> > By just removing the define this is less of an issue, but you should
> > not make the life of reviewers harder by ignoring request to properly
> > motivate your changes and explain why they are safe to apply.
> 
> My apologies again, thanks for the patience.
> 
> > You can either put a shared changelog for the whole series in a cover
> > letter, or you describe changes to each individual patch. But mixing the
> > two styles as you do in this entry is just confusing.
> 
> Thanks for the suggestions. In order to simplify the submission process,
> i propose to split the submission for the LARA-L6 patches and the update
> for LARA-R6 00B.
> Do you think could it be feasible?

I don't think that's necessary now that you've provided some more
details. Just respin the series and address the review comments given
so far (either by rejecting a suggestion and explaining why, or by
incorporating it in your next submission).

It seems you only need to tweak some of the commit messages in a v6.

Johan



[Index of Archives]     [Linux Media]     [Linux Input]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [Old Linux USB Devel Archive]

  Powered by Linux