On Mon, Sep 09, 2024 at 02:11:50PM +0200, Hans de Goede wrote:
> On 9/9/24 2:06 PM, Andy Shevchenko wrote:
> > On Mon, Sep 09, 2024 at 01:32:25PM +0200, Hans de Goede wrote:
...
> >> +static umode_t pcc_sysfs_is_visible(struct kobject *kobj, struct attribute *attr, int idx)
> >> +{
> >> + struct device *dev = kobj_to_dev(kobj);
> >> + struct acpi_device *acpi = to_acpi_device(dev);
> >> + struct pcc_acpi *pcc = acpi_driver_data(acpi);
> >
> > Isn't it the same as dev_get_drvdata()?
>
> No I also thought so and I checked. It is not the same,
> struct acpi_device has its own driver_data member, which
> this gets.
Ouch, you are right! It's a bit confusing :-(
> >> + if (attr == &dev_attr_mute.attr)
> >> + return (pcc->num_sifr > SINF_MUTE) ? attr->mode : 0;
> >> +
> >> + if (attr == &dev_attr_eco_mode.attr)
> >> + return (pcc->num_sifr > SINF_ECO_MODE) ? attr->mode : 0;
> >> +
> >> + if (attr == &dev_attr_current_brightness.attr)
> >> + return (pcc->num_sifr > SINF_CUR_BRIGHT) ? attr->mode : 0;
> >> +
> >> + return attr->mode;
> >> +}
...
> >> + /*
> >> + * pcc->sinf is expected to at least have the AC+DC brightness entries.
> >> + * Accesses to higher SINF entries are checked against num_sifr.
> >> + */
> >> + if (num_sifr <= SINF_DC_CUR_BRIGHT || num_sifr > 255) {
> >> + pr_err("num_sifr %d out of range %d - 255\n", num_sifr, SINF_DC_CUR_BRIGHT + 1);
> >
> > acpi_handle_err() ?
>
> The driver is using pr_err() already in 18 other places, so IMHO it is better
> to be consistent and also use it here.
Fair enough, so can be material for a followup in the future.
--
With Best Regards,
Andy Shevchenko
