RE: [PATCH v4 1/2] scsi: ufs: Probe for temperature notification support

[Avri Altman <Avri.Altman@wdc.com>]

> > +
> > +static bool ufs_read_temp_enable(struct ufs_hba *hba, u8 mask) {
> > +     u32 ee_mask;
> > +
> > +     if (ufshcd_query_attr(hba, UPIU_QUERY_OPCODE_READ_ATTR,
> QUERY_ATTR_IDN_EE_CONTROL, 0, 0,
> > +                           &ee_mask))
> > +             return false;
> 
> That should probably return the error code from ufshcd_query_attr(). I don't
> see a good reason to ignore it.
Done.

> 
> > +
> > +     return (mask & ee_mask & MASK_EE_TOO_HIGH_TEMP) || (mask &
> > +ee_mask & MASK_EE_TOO_LOW_TEMP); }
> > +
> > +static int ufs_get_temp(struct ufs_hba *hba, enum attr_idn idn, long
> > +*val) {
> > +     u32 value;
> > +
> > +     if (ufshcd_query_attr(hba, UPIU_QUERY_OPCODE_READ_ATTR, idn, 0,
> 0, &value))
> > +             return -EINVAL;
> 
> ufshcd_query_attr() already returns a valid Linux error code. It should be
> returned and not be overwritten. Besides, -EINVAL is wrong in most cases. It
> is only an acceptable error code if parameters passed to ufshcd_query_attr()
> are wrong, which is surely not the case here.
Done.

> 
> > +
> > +     if (value == 0)
> > +             return -EINVAL;
> 
> As mentioned in my other response, this is expected if the sensor is
> disabled, and thus should return -ENODATA.
Done.

Thanks,
Avri