Hello Laurent, On 5/2/22 20:01, Laurent Pinchart wrote: > Hi Javier, [snip] >>>> + fb_helper->firmware = firmware; >>> >>> I'd get rid of the local variable and write >>> >> >> I actually considered that but then decided to add a local variable to >> have both options set in the same place, since I thought that would be >> easier to read and also consistent with how preferred_bpp is handled. >> >> Maybe I could go the other way around and rework patch 2/3 to also not >> require a preferred_bpp local variable ? That patch won't be as small >> as it's now though. -- > > Up to you, or you could ignore the comment, it's minor. If you want to > keep the variable, you could also make it const, it's a good practice to > show it isn't intended to be modified. > Right. I'll also do the same for the preferred_bpp variable in patch 2/3 if I choose to keep them in v3. Thanks again for your feedback and comments! -- Best regards, Javier Martinez Canillas Linux Engineering Red Hat
