On Tue, Aug 9, 2022 at 5:30 AM Luke D. Jones <luke@xxxxxxxxxx> wrote: > > Due to multiple types of tablet/lidflip, the existing code for > handling these events is refactored to use an enum for each type. ... > static int asus_wmi_input_init(struct asus_wmi *asus) > { > + struct device *dev; > int err, result; > > + dev = &asus->platform_device->dev; > + While the discussed pattern of splitting assignments is a good practice, for some cases we don't do it when we rely on the guarantee by the callers that some of the stuff won't be problematic. Here the device is part of the platform device and can't be NULL, there is no point to split definition and assignment (and you may find plenty examples in the kernel), so struct device *dev = &asus->platform_device->dev; is better to have as it's guaranteed not to be NULL and since that we don't check it in the code anyway. ... > input_report_switch(asus->inputdev, SW_TABLET_MODE, > - !result); > + !result); Irrelevant change. ... It also seems you switched to dev_err() here for the pr_err() that aren't related to the change, either split that to a separate patch, or don't do it right now. I.o.w. use dev_err() only in the lines your change touches, otherwise it's irrelevant. -- With Best Regards, Andy Shevchenko