| Message ID | 1496790041-104500-1-git-send-email-srinivas.pandruvada@linux.intel.com (mailing list archive) |
|---|---|
| State | Accepted, archived |
| Delegated to: | Zhang Rui |
| Headers | show |
On Wed, Jun 7, 2017 at 2:00 AM, Srinivas Pandruvada <srinivas.pandruvada@linux.intel.com> wrote: > For INT3403 sensor PTYP field is mandatory. But some platforms didn't > have this field for sensors. This cause load failure for int3403 driver. > > This change checks for the presence of _TMP method and if present, then > treats this device as a sensor. > status = acpi_evaluate_integer(priv->adev->handle, "PTYP", > NULL, &priv->type); > if (ACPI_FAILURE(status)) { > - result = -EINVAL; > - goto err; > + unsigned long long tmp; You may use &priv->type as temporary variable, though I would go other way around: declare tmp for function, then unsigned long long tmp; ... status = acpi_evaluate_integer(priv->adev->handle, "PTYP", NULL, &tmp); if (ACPI_FAILURE(status)) { status = acpi_evaluate_integer(priv->adev->handle, "_TMP", NULL, &tmp); if (ACPI_FAILURE(status)) { result = -EINVAL; goto err; } tmp = INT3403_TYPE_SENSOR; } priv->type = tmp; > + } else { This is redundant. > + priv->type = INT3403_TYPE_SENSOR; > + } > }
On Wed, 2017-06-07 at 13:55 +0300, Andy Shevchenko wrote: > On Wed, Jun 7, 2017 at 2:00 AM, Srinivas Pandruvada > <srinivas.pandruvada@linux.intel.com> wrote: > > > > For INT3403 sensor PTYP field is mandatory. But some platforms > > didn't > > have this field for sensors. This cause load failure for int3403 > > driver. > > > > This change checks for the presence of _TMP method and if present, > > then > > treats this device as a sensor. > > > > status = acpi_evaluate_integer(priv->adev->handle, "PTYP", > > NULL, &priv->type); > > if (ACPI_FAILURE(status)) { > > - result = -EINVAL; > > - goto err; > > > > + unsigned long long tmp; > You may use &priv->type as temporary variable, though I would go > other > way around: > declare tmp for function, then > > unsigned long long tmp; > ... > status = acpi_evaluate_integer(priv->adev->handle, "PTYP", NULL, > &tmp); > if (ACPI_FAILURE(status)) { > status = acpi_evaluate_integer(priv->adev->handle, "_TMP", > NULL, &tmp); > if (ACPI_FAILURE(status)) { > result = -EINVAL; > goto err; > } > tmp = INT3403_TYPE_SENSOR; > } > priv->type = tmp; > So what are we saving by doing this way? Thanks, Srinivas > > > > + } else { > This is redundant. > > > > > + priv->type = INT3403_TYPE_SENSOR; > > + } > > }
On Sat, Jun 10, 2017 at 1:23 AM, Srinivas Pandruvada <srinivas.pandruvada@linux.intel.com> wrote: > On Wed, 2017-06-07 at 13:55 +0300, Andy Shevchenko wrote: >> On Wed, Jun 7, 2017 at 2:00 AM, Srinivas Pandruvada >> <srinivas.pandruvada@linux.intel.com> wrote: >> > This change checks for the presence of _TMP method and if present, >> > then >> > treats this device as a sensor. >> > >> > status = acpi_evaluate_integer(priv->adev->handle, "PTYP", >> > NULL, &priv->type); >> > if (ACPI_FAILURE(status)) { >> > - result = -EINVAL; >> > - goto err; >> > >> > + unsigned long long tmp; >> You may use &priv->type as temporary variable, though I would go >> other >> way around: >> declare tmp for function, then >> >> unsigned long long tmp; >> ... >> status = acpi_evaluate_integer(priv->adev->handle, "PTYP", NULL, >> &tmp); >> if (ACPI_FAILURE(status)) { >> status = acpi_evaluate_integer(priv->adev->handle, "_TMP", >> NULL, &tmp); >> if (ACPI_FAILURE(status)) { >> result = -EINVAL; >> goto err; >> } >> tmp = INT3403_TYPE_SENSOR; >> } >> priv->type = tmp; >> > So what are we saving by doing this way? We make priv->type assignment in one place. That's the difference. So, you may choose your way of course. It's not a big deal.
diff --git a/drivers/thermal/int340x_thermal/int3403_thermal.c b/drivers/thermal/int340x_thermal/int3403_thermal.c index c4890c9..8a7f24d 100644 --- a/drivers/thermal/int340x_thermal/int3403_thermal.c +++ b/drivers/thermal/int340x_thermal/int3403_thermal.c @@ -238,8 +238,16 @@ static int int3403_add(struct platform_device *pdev) status = acpi_evaluate_integer(priv->adev->handle, "PTYP", NULL, &priv->type); if (ACPI_FAILURE(status)) { - result = -EINVAL; - goto err; + unsigned long long tmp; + + status = acpi_evaluate_integer(priv->adev->handle, "_TMP", + NULL, &tmp); + if (ACPI_FAILURE(status)) { + result = -EINVAL; + goto err; + } else { + priv->type = INT3403_TYPE_SENSOR; + } } platform_set_drvdata(pdev, priv);
For INT3403 sensor PTYP field is mandatory. But some platforms didn't have this field for sensors. This cause load failure for int3403 driver. This change checks for the presence of _TMP method and if present, then treats this device as a sensor. Signed-off-by: Srinivas Pandruvada <srinivas.pandruvada@linux.intel.com> --- drivers/thermal/int340x_thermal/int3403_thermal.c | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-)