Hello Johan, Greg
On Fri, Dec 15, 2023 at 05:18:52PM +0100, Johan Hovold wrote:
On Fri, Dec 15, 2023 at 02:55:59PM +0100, Francesco Dolcini wrote:
To me the change is correct, with that said probably this should have been explicitly mentioned in the commit message or a separate preparation patch.
It's a separate change and should not be hidden away in a tree-wide change that goes through a different maintainer.
Please drop this change from this patch and resubmit it separately to me if you want and I'll review when I have the time.
Fine, I agree.
I see those options (let me know if you see other options I have not mentioned):
1. I add this change (taking into account also intel ice) as a separate patch in this series and you may just ack it and Greg could merge together with the serdev one. 2. I prepare an independent patch for the GNSS change and only once this is merged I'll send a rebased v2 of this one. 3. I update this patch without this GNSS API change, that mean I will have to cast away the signed type from a few GNSS drivers.
1 is my preferred option, 2 is fine, but it seems a little bit of overdoing, 3 I would avoid, we are doing this cleanup to be a little bit more strongly typed and to prevent the kind of bugs that is the original trigger for this patch.
What would you Greg and Johan prefer?
And when doing tree-wide changes, please try to follow the style of the driver you are changing (e.g. do not introduce inconsistencies by changing to open parenthesis alignment of continuation lines in code that do not use it).
ack, sorry about that, looking back at the archive is seems a recent pain point, also Jiri fell in this trap.
Francesco