Re: [PATCH v3 2/2] hwmon: (ltq-cputemp) add devicetree bindings documentation
On Thu, Sep 14, 2017 at 2:06 AM, Florian Eckert wrote:
> Hello Rob
>
> > +
> > +Requires node properties:
> > +- compatible value :
> > + "lantiq,cputemp"
Kind of non-specific. How is this device even accessed without any other
property?
>>>
>>>
>>>
>>> It does not need any further properties. If this is set in the device
>>> tree
>>> then the driver is loaded.
>>
>>
>> DT is not the only way to instantiate drivers.
>>
>> What I meant is how do you access the hardware? That should be evident
>> from the binding and it is not.
>
>
> Agree with our statement.
>
>>
>> Looking at the driver, you have some memory mapped system control
>> registers which get ioremapped in arch/mips/lantiq/xway/sysctrl.c and
>> accesses thru some platform specific macros. That is not the ideal way
>> to do things as we use syscon and regmap for such things. But that's
>> all mostly kernel details not so relevant to the DT binding.
>
>
> For lanitq xrx200 this is all i have. So if i have to use syscon and regmap
> i am also not sure how to handle this.
>
>> For DT, I'd expect this is a child node of the sysctrl block with a
>> reg property value of <0x40 4> (along with any other child devices).
>> You could also not even put this in DT and the system controller can
>> have it's own driver that instantiates the child device for this
>> driver.
>
>
> Yes this would be the best practice. But the hardware designer for what ever
> reason
> placed the Register for the temperature sensors into the CGU (Clock
> Generation Unit) section!
> And the Register is also shared with some other stuff which is not only
> assign for temperature
> stuff! I am not sure how to handle this in the device tree.
This is quite common and there are plenty of examples. Look for
bindings with "syscon".
>
> This is a Register description extract from the data sheet
>
> GPHY Configuration Register 01
> This register configures the booting options of GPHY1 IP.
>
> Offset 0x0040
> Reset Value 0x01FC
>
> 31: RES
> 30: 100FX_H
> 29: 100FX_F
> 28: 10BT_F
> 27: 10BT_H
> 26: 100BT_F
> 25: 100BT_H
> 24: 1000BT_F
> 23: 1000BT_H
> 22: RES
> 21: RES
> 19: TEMP_PD <--- NEEDED Power down the Temperature Sensor
> 18: TEMP_HL <--- NEEDED Indicate temperature higher than 128 C
> 17: TEMP<--- NEEDED Value
> 16: TEMP<--- NEEDED Value
> 15: TEMP<--- NEEDED Value
> 14: TEMP<--- NEEDED Value
> 13: TEMP<--- NEEDED Value
> 12: TEMP<--- NEEDED Value
> 11: TEMP<--- NEEDED Value
> 10: TEMP<--- NEEDED Value
> 09: TEMP<--- NEEDED Value
> 08: RES
> 07: RES
> 06: SPI_Delay
> 05: SPI_Delay
> 04: AHB_EnD
> 03: DMA_OR
> 02: RES
> 01: RES
> 00: RES
>
> And that is a dts tree from LEDE/Openwrt for lantiq
> URL:
> https://github.com/lede-project/source/blob/c88770c766fdc5599efc4672bca230017f52e8e4/target/linux/lantiq/dts/vr9.dtsi#L54
>
> Extraction:
> sram@1F00 {
> #address-cells = <1>;
> #size-cells = <1>;
> compatible = "lantiq,sram", "simple-bus";
> reg = <0x1F00 0x80>;
> ranges = <0x0 0x1F00 0x7F>;
>
> eiu0: eiu@101000 {
> #interrupt-cells = <1>;
> interrupt-controller;
> compatible = "lantiq,eiu-xway";
> reg = <0x101000 0x1000>;
> interrupt-parent = <&icu0>;
> lantiq,eiu-irqs = <166 135 66 40 41 42>;
> };
>
> pmu0: pmu@102000 {
> compatible = "lantiq,pmu-xway";
> reg = <0x102000 0x1000>;
> };
>
> cgu0: cgu@103000 {
> compatible = "lantiq,cgu-xway";
> reg = <0x103000 0x1000>;
> -> This is the place to add the binding?
Yes.
> };
>
> Sorry for the noise but i am unsure how to do this.
> Thanks for help
>
> Florian
--
To unsubscribe from this list: send the line "unsubscribe linux-hwmon" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2/2] hwmon: add dt binding for max1619
On Tue, Sep 19, 2017 at 1:11 PM, Alan Tull wrote: > On Mon, Sep 18, 2017 at 4:11 PM, Rob Herring wrote: >> On Mon, Sep 11, 2017 at 12:58:57PM -0700, Guenter Roeck wrote: >>> On Mon, Sep 11, 2017 at 02:16:49PM -0500, Alan Tull wrote: >>> > Add new device tree binding for max1619. >>> > >>> > Signed-off-by: Alan Tull >>> >>> Technically that should already work, without explicit binding, >>> or did the i2c core change lately ? >> >> There was some work in that direction IIRC. > > Yes, after Guenter replied I tried it without this patch and it > worked. i2c drivers can have a 'detect' function that can check part > id, etc. If detection succeeds, it will fill in the device name in > i2c_board_info->type and the i2c-core will enumerate using that. Right, I expected it still worked, but am saying we don't want to rely on that behavior and introduce new places relying on it. Otherwise, the driver will match on say "rob,max1619" as well. The detect function is the really old way to probe devices IIRC. The function to look at is i2c_of_match_device. We want OF style match, not a match with i2c_of_match_device_sysfs which strips the vendor prefix from the compatible. Rob -- To unsubscribe from this list: send the line "unsubscribe linux-hwmon" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2/2] hwmon: add dt binding for max1619
On Mon, Sep 18, 2017 at 4:11 PM, Rob Herring wrote: > On Mon, Sep 11, 2017 at 12:58:57PM -0700, Guenter Roeck wrote: >> On Mon, Sep 11, 2017 at 02:16:49PM -0500, Alan Tull wrote: >> > Add new device tree binding for max1619. >> > >> > Signed-off-by: Alan Tull >> >> Technically that should already work, without explicit binding, >> or did the i2c core change lately ? > > There was some work in that direction IIRC. Yes, after Guenter replied I tried it without this patch and it worked. i2c drivers can have a 'detect' function that can check part id, etc. If detection succeeds, it will fill in the device name in i2c_board_info->type and the i2c-core will enumerate using that. Alan > > We don't really want to rely on that behavior and should have DT match > strings. > > Rob -- To unsubscribe from this list: send the line "unsubscribe linux-hwmon" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] hwmon: (stts751) Fix buffer size passed to snprintf
On Tue, Sep 19, 2017 at 02:59:34PM +0200, Jean Delvare wrote:
> Function snprintf already cares for the terminating NUL at the end of
> the string, the caller doesn't need to do it.
>
> Signed-off-by: Jean Delvare
> Cc: Andrea Merello
> Cc: Guenter Roeck
Applied.
Thanks,
Guenter
> ---
> drivers/hwmon/stts751.c | 18 +-
> 1 file changed, 9 insertions(+), 9 deletions(-)
>
> --- linux-4.14-rc1.orig/drivers/hwmon/stts751.c 2017-09-17
> 00:47:51.0 +0200
> +++ linux-4.14-rc1/drivers/hwmon/stts751.c2017-09-19 14:51:39.773843565
> +0200
> @@ -396,7 +396,7 @@ static ssize_t show_max_alarm(struct dev
> if (ret < 0)
> return ret;
>
> - return snprintf(buf, PAGE_SIZE - 1, "%d\n", priv->max_alert);
> + return snprintf(buf, PAGE_SIZE, "%d\n", priv->max_alert);
> }
>
> static ssize_t show_min_alarm(struct device *dev, struct device_attribute
> *attr,
> @@ -413,7 +413,7 @@ static ssize_t show_min_alarm(struct dev
> if (ret < 0)
> return ret;
>
> - return snprintf(buf, PAGE_SIZE - 1, "%d\n", priv->min_alert);
> + return snprintf(buf, PAGE_SIZE, "%d\n", priv->min_alert);
> }
>
> static ssize_t show_input(struct device *dev, struct device_attribute *attr,
> @@ -428,7 +428,7 @@ static ssize_t show_input(struct device
> if (ret < 0)
> return ret;
>
> - return snprintf(buf, PAGE_SIZE - 1, "%d\n", priv->temp);
> + return snprintf(buf, PAGE_SIZE, "%d\n", priv->temp);
> }
>
> static ssize_t show_therm(struct device *dev, struct device_attribute *attr,
> @@ -436,7 +436,7 @@ static ssize_t show_therm(struct device
> {
> struct stts751_priv *priv = dev_get_drvdata(dev);
>
> - return snprintf(buf, PAGE_SIZE - 1, "%d\n", priv->therm);
> + return snprintf(buf, PAGE_SIZE, "%d\n", priv->therm);
> }
>
> static ssize_t set_therm(struct device *dev, struct device_attribute *attr,
> @@ -478,7 +478,7 @@ static ssize_t show_hyst(struct device *
> {
> struct stts751_priv *priv = dev_get_drvdata(dev);
>
> - return snprintf(buf, PAGE_SIZE - 1, "%d\n", priv->hyst);
> + return snprintf(buf, PAGE_SIZE, "%d\n", priv->hyst);
> }
>
> static ssize_t set_hyst(struct device *dev, struct device_attribute *attr,
> @@ -518,7 +518,7 @@ static ssize_t show_therm_trip(struct de
> if (ret < 0)
> return ret;
>
> - return snprintf(buf, PAGE_SIZE - 1, "%d\n", priv->therm_trip);
> + return snprintf(buf, PAGE_SIZE, "%d\n", priv->therm_trip);
> }
>
> static ssize_t show_max(struct device *dev, struct device_attribute *attr,
> @@ -526,7 +526,7 @@ static ssize_t show_max(struct device *d
> {
> struct stts751_priv *priv = dev_get_drvdata(dev);
>
> - return snprintf(buf, PAGE_SIZE - 1, "%d\n", priv->event_max);
> + return snprintf(buf, PAGE_SIZE, "%d\n", priv->event_max);
> }
>
> static ssize_t set_max(struct device *dev, struct device_attribute *attr,
> @@ -560,7 +560,7 @@ static ssize_t show_min(struct device *d
> {
> struct stts751_priv *priv = dev_get_drvdata(dev);
>
> - return snprintf(buf, PAGE_SIZE - 1, "%d\n", priv->event_min);
> + return snprintf(buf, PAGE_SIZE, "%d\n", priv->event_min);
> }
>
> static ssize_t set_min(struct device *dev, struct device_attribute *attr,
> @@ -594,7 +594,7 @@ static ssize_t show_interval(struct devi
> {
> struct stts751_priv *priv = dev_get_drvdata(dev);
>
> - return snprintf(buf, PAGE_SIZE - 1, "%d\n",
> + return snprintf(buf, PAGE_SIZE, "%d\n",
> stts751_intervals[priv->interval]);
> }
>
>
>
> --
> Jean Delvare
> SUSE L3 Support
> --
> To unsubscribe from this list: send the line "unsubscribe linux-hwmon" in
> the body of a message to majord...@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
--
To unsubscribe from this list: send the line "unsubscribe linux-hwmon" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] hwmon: (stts751) Fix buffer size passed to snprintf
Function snprintf already cares for the terminating NUL at the end of
the string, the caller doesn't need to do it.
Signed-off-by: Jean Delvare
Cc: Andrea Merello
Cc: Guenter Roeck
---
drivers/hwmon/stts751.c | 18 +-
1 file changed, 9 insertions(+), 9 deletions(-)
--- linux-4.14-rc1.orig/drivers/hwmon/stts751.c 2017-09-17 00:47:51.0
+0200
+++ linux-4.14-rc1/drivers/hwmon/stts751.c 2017-09-19 14:51:39.773843565
+0200
@@ -396,7 +396,7 @@ static ssize_t show_max_alarm(struct dev
if (ret < 0)
return ret;
- return snprintf(buf, PAGE_SIZE - 1, "%d\n", priv->max_alert);
+ return snprintf(buf, PAGE_SIZE, "%d\n", priv->max_alert);
}
static ssize_t show_min_alarm(struct device *dev, struct device_attribute
*attr,
@@ -413,7 +413,7 @@ static ssize_t show_min_alarm(struct dev
if (ret < 0)
return ret;
- return snprintf(buf, PAGE_SIZE - 1, "%d\n", priv->min_alert);
+ return snprintf(buf, PAGE_SIZE, "%d\n", priv->min_alert);
}
static ssize_t show_input(struct device *dev, struct device_attribute *attr,
@@ -428,7 +428,7 @@ static ssize_t show_input(struct device
if (ret < 0)
return ret;
- return snprintf(buf, PAGE_SIZE - 1, "%d\n", priv->temp);
+ return snprintf(buf, PAGE_SIZE, "%d\n", priv->temp);
}
static ssize_t show_therm(struct device *dev, struct device_attribute *attr,
@@ -436,7 +436,7 @@ static ssize_t show_therm(struct device
{
struct stts751_priv *priv = dev_get_drvdata(dev);
- return snprintf(buf, PAGE_SIZE - 1, "%d\n", priv->therm);
+ return snprintf(buf, PAGE_SIZE, "%d\n", priv->therm);
}
static ssize_t set_therm(struct device *dev, struct device_attribute *attr,
@@ -478,7 +478,7 @@ static ssize_t show_hyst(struct device *
{
struct stts751_priv *priv = dev_get_drvdata(dev);
- return snprintf(buf, PAGE_SIZE - 1, "%d\n", priv->hyst);
+ return snprintf(buf, PAGE_SIZE, "%d\n", priv->hyst);
}
static ssize_t set_hyst(struct device *dev, struct device_attribute *attr,
@@ -518,7 +518,7 @@ static ssize_t show_therm_trip(struct de
if (ret < 0)
return ret;
- return snprintf(buf, PAGE_SIZE - 1, "%d\n", priv->therm_trip);
+ return snprintf(buf, PAGE_SIZE, "%d\n", priv->therm_trip);
}
static ssize_t show_max(struct device *dev, struct device_attribute *attr,
@@ -526,7 +526,7 @@ static ssize_t show_max(struct device *d
{
struct stts751_priv *priv = dev_get_drvdata(dev);
- return snprintf(buf, PAGE_SIZE - 1, "%d\n", priv->event_max);
+ return snprintf(buf, PAGE_SIZE, "%d\n", priv->event_max);
}
static ssize_t set_max(struct device *dev, struct device_attribute *attr,
@@ -560,7 +560,7 @@ static ssize_t show_min(struct device *d
{
struct stts751_priv *priv = dev_get_drvdata(dev);
- return snprintf(buf, PAGE_SIZE - 1, "%d\n", priv->event_min);
+ return snprintf(buf, PAGE_SIZE, "%d\n", priv->event_min);
}
static ssize_t set_min(struct device *dev, struct device_attribute *attr,
@@ -594,7 +594,7 @@ static ssize_t show_interval(struct devi
{
struct stts751_priv *priv = dev_get_drvdata(dev);
- return snprintf(buf, PAGE_SIZE - 1, "%d\n",
+ return snprintf(buf, PAGE_SIZE, "%d\n",
stts751_intervals[priv->interval]);
}
--
Jean Delvare
SUSE L3 Support
--
To unsubscribe from this list: send the line "unsubscribe linux-hwmon" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
