Re: [PATCH v3 2/5] w1_therm: adding sysfs entry to check device power

[Evgeniy Polyakov]

Hi

29.04.2020, 16:47, "Greg KH" :

>>  +What: /sys/bus/w1/devices/.../w1_slave
>>  +Date: Apr 2020
>>  +Contact: Akira Shimahara 
>>  +Description:
>>  + (RW) return the temperature in 1/1000 degC.
>>  + *read*: return 2 lines with the hexa output data sent on the
>>  + bus, return the CRC check and temperature in 1/1000 degC
>
> the w1_slave file returns a temperature???
>
> And sysfs is 1 value-per-file, not multiple lines.

It was 'content crc' previously, and probably a good idea would be to add just 
one file with 'content'

> And as this is a temperature, what's wrong with the iio interface that
> handles temperature already? Don't go making up new userspace apis when
> we already have good ones today :)

What is that?
w1 always had a sysfs files for its contents whether it is converted 
temperature or raw bytes data,
there is also netlink interface which is there since the day one. 

>>  + *write* :
>>  + * `0` : save the 2 or 3 bytes to the device EEPROM
>>  + (i.e. TH, TL and config register)
>>  + * `9..12` : set the device resolution in RAM
>>  + (if supported)
>
> I don't understand these write values, how do they match up to a
> temperature readin?

You kind of writing to device about how to convert its raw content into 
readable content, which will eventually become a temperature

Re: [PATCH 2/2] w1: fix the resume command API

[Evgeniy Polyakov]

Hi everyone

Mariusz, thank you for the patch, a small comment on it logic

19.03.2019, 16:21, "Jean-Francois Dagenais" :

>>  ---
>>  drivers/w1/w1_io.c | 11 +--
>>  1 file changed, 9 insertions(+), 2 deletions(-)
>>
>>  diff --git a/drivers/w1/w1_io.c b/drivers/w1/w1_io.c
>>  index 0364d3329c52..4697136b9027 100644
>>  --- a/drivers/w1/w1_io.c
>>  +++ b/drivers/w1/w1_io.c
>>  @@ -432,8 +432,15 @@ int w1_reset_resume_command(struct w1_master *dev)
>>  if (w1_reset_bus(dev))
>>  return -1;
>>
>>  - /* This will make only the last matched slave perform a skip ROM. */
>>  - w1_write_8(dev, W1_RESUME_CMD);
>>  + if (dev->slave_count == 1) {
>>  + /* Resume Command has to be preceeded with e.g. Match ROM which is
>>  + * not happening on single-slave buses, just do a Skip ROM instead
>>  + */
>>  + w1_write_8(dev, W1_SKIP_ROM);

Looks like this may break the search logic, can you check that with this patch 
applied
some other single slave device will correctly 'tell' its id and it can be 
addressed via match rom (like, basically, just reading temperature or something 
like that)?

>>  + } else {
>>  + /* This will make only the last matched slave perform a skip ROM. */
>>  + w1_write_8(dev, W1_RESUME_CMD);
>>  + }
>
> This may be a subsys maintainer's style preference, but perhaps the verbose 
> comments
> might be better suited for the git commit message. Could this then be shorted 
> to
>
> if (dev->slave_count == 1)
> w1_write_8(dev, W1_SKIP_ROM);
> else
> w1_write_8(dev, W1_RESUME_CMD);
>
> Or maybe:
>
> w1_write_8(dev, dev->slave_count > 1 ? W1_RESUME_CMD : W1_SKIP_ROM);
>
> I am also ok with this proposed version, hence the "reviewed-by".
>
>>  return 0;
>>  }
>>  EXPORT_SYMBOL_GPL(w1_reset_resume_command);
>>  --
>>  2.19.0.rc1

Re: [PATCH 18/18] w1: w1_io.c: fix a kernel-doc warning

[Evgeniy Polyakov]

Hi

09.05.2018, 16:11, "Jonathan Corbet" <cor...@lwn.net>:
> On Wed, 9 May 2018 09:32:18 -0300
> Mauro Carvalho Chehab <mchehab+sams...@kernel.org> wrote:
>
>>  > Looks good to me, thank you!
>>  > What tree should this be forwarded to, or folks from linux doc will pick 
>> it up?
>>  >
>>  > Acked-by: Evgeniy Polyakov <z...@ioremap.net>
>>
>>  Jon prefers if this could go via the usual maintainer's tree.
>>
>>  So, could you send it via yours?
>
> With the ack I can pick it up, no worries. I somehow missed this when I
> went through the set.

Great, thank you!
Let me know if you want me to push it upstream via my tree.

Re: [PATCH 18/18] w1: w1_io.c: fix a kernel-doc warning

[Evgeniy Polyakov]

Hi

09.05.2018, 16:11, "Jonathan Corbet" :
> On Wed, 9 May 2018 09:32:18 -0300
> Mauro Carvalho Chehab  wrote:
>
>>  > Looks good to me, thank you!
>>  > What tree should this be forwarded to, or folks from linux doc will pick 
>> it up?
>>  >
>>  > Acked-by: Evgeniy Polyakov 
>>
>>  Jon prefers if this could go via the usual maintainer's tree.
>>
>>  So, could you send it via yours?
>
> With the ack I can pick it up, no worries. I somehow missed this when I
> went through the set.

Great, thank you!
Let me know if you want me to push it upstream via my tree.

Re: [PATCH 18/18] w1: w1_io.c: fix a kernel-doc warning

[Evgeniy Polyakov]

Hi

07.05.2018, 12:36, "Mauro Carvalho Chehab" <mchehab+sams...@kernel.org>:
> Add a blank line to avoid this Sphinx warning:
> ./drivers/w1/w1_io.c:197: WARNING: Definition list ends without a 
> blank line; unexpected unindent.
>
> Signed-off-by: Mauro Carvalho Chehab <mchehab+sams...@kernel.org>

Looks good to me, thank you!
What tree should this be forwarded to, or folks from linux doc will pick it up?

Acked-by: Evgeniy Polyakov <z...@ioremap.net>

> ---
>  drivers/w1/w1_io.c | 1 +
>  1 file changed, 1 insertion(+)
>
> diff --git a/drivers/w1/w1_io.c b/drivers/w1/w1_io.c
> index 075d120e7b88..0364d3329c52 100644
> --- a/drivers/w1/w1_io.c
> +++ b/drivers/w1/w1_io.c
> @@ -194,6 +194,7 @@ static u8 w1_read_bit(struct w1_master *dev)
>   * bit 0 = id_bit
>   * bit 1 = comp_bit
>   * bit 2 = dir_taken
> + *
>   * If both bits 0 & 1 are set, the search should be restarted.
>   *
>   * Return: bit fields - see above
> --
> 2.17.0

Re: [PATCH 18/18] w1: w1_io.c: fix a kernel-doc warning

[Evgeniy Polyakov]

Hi

07.05.2018, 12:36, "Mauro Carvalho Chehab" :
> Add a blank line to avoid this Sphinx warning:
> ./drivers/w1/w1_io.c:197: WARNING: Definition list ends without a 
> blank line; unexpected unindent.
>
> Signed-off-by: Mauro Carvalho Chehab 

Looks good to me, thank you!
What tree should this be forwarded to, or folks from linux doc will pick it up?

Acked-by: Evgeniy Polyakov 

> ---
>  drivers/w1/w1_io.c | 1 +
>  1 file changed, 1 insertion(+)
>
> diff --git a/drivers/w1/w1_io.c b/drivers/w1/w1_io.c
> index 075d120e7b88..0364d3329c52 100644
> --- a/drivers/w1/w1_io.c
> +++ b/drivers/w1/w1_io.c
> @@ -194,6 +194,7 @@ static u8 w1_read_bit(struct w1_master *dev)
>   * bit 0 = id_bit
>   * bit 1 = comp_bit
>   * bit 2 = dir_taken
> + *
>   * If both bits 0 & 1 are set, the search should be restarted.
>   *
>   * Return: bit fields - see above
> --
> 2.17.0

Re: [PATCH v2] w1: mxc_w1: Enable clock before calling clk_get_rate() on it

[Evgeniy Polyakov]

Hi Stefan

02.05.2018, 11:55, "Stefan Potyra" <stefan.pot...@elektrobit.com>:
> According to the API, you may only call clk_get_rate() after actually
> enabling it.
>
> Found by Linux Driver Verification project (linuxtesting.org).
>
> Fixes: a5fd9139f74c ("w1: add 1-wire master driver for i.MX27 / i.MX31")
> Signed-off-by: Stefan Potyra <stefan.pot...@elektrobit.com>

Looks good to me, thank you!
Greg, please pull it into your tree, this also sounds like stable material, is 
it?

Acked-by: Evgeniy Polyakov <z...@ioremap.net>

> ---
>  drivers/w1/masters/mxc_w1.c | 20 +---
>  1 file changed, 13 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/w1/masters/mxc_w1.c b/drivers/w1/masters/mxc_w1.c
> index 74f2e6e6202a..8851d441e5fd 100644
> --- a/drivers/w1/masters/mxc_w1.c
> +++ b/drivers/w1/masters/mxc_w1.c
> @@ -112,6 +112,10 @@ static int mxc_w1_probe(struct platform_device *pdev)
>  if (IS_ERR(mdev->clk))
>  return PTR_ERR(mdev->clk);
>
> + err = clk_prepare_enable(mdev->clk);
> + if (err)
> + return err;
> +
>  clkrate = clk_get_rate(mdev->clk);
>  if (clkrate < 1000)
>  dev_warn(>dev,
> @@ -125,12 +129,10 @@ static int mxc_w1_probe(struct platform_device *pdev)
>
>  res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
>  mdev->regs = devm_ioremap_resource(>dev, res);
> - if (IS_ERR(mdev->regs))
> - return PTR_ERR(mdev->regs);
> -
> - err = clk_prepare_enable(mdev->clk);
> - if (err)
> - return err;
> + if (IS_ERR(mdev->regs)) {
> + err = PTR_ERR(mdev->regs);
> + goto out_disable_clk;
> + }
>
>  /* Software reset 1-Wire module */
>  writeb(MXC_W1_RESET_RST, mdev->regs + MXC_W1_RESET);
> @@ -146,8 +148,12 @@ static int mxc_w1_probe(struct platform_device *pdev)
>
>  err = w1_add_master_device(>bus_master);
>  if (err)
> - clk_disable_unprepare(mdev->clk);
> + goto out_disable_clk;
>
> + return 0;
> +
> +out_disable_clk:
> + clk_disable_unprepare(mdev->clk);
>  return err;
>  }
>
> --
> 2.17.0

Re: [PATCH v2] w1: mxc_w1: Enable clock before calling clk_get_rate() on it

[Evgeniy Polyakov]

Hi Stefan

02.05.2018, 11:55, "Stefan Potyra" :
> According to the API, you may only call clk_get_rate() after actually
> enabling it.
>
> Found by Linux Driver Verification project (linuxtesting.org).
>
> Fixes: a5fd9139f74c ("w1: add 1-wire master driver for i.MX27 / i.MX31")
> Signed-off-by: Stefan Potyra 

Looks good to me, thank you!
Greg, please pull it into your tree, this also sounds like stable material, is 
it?

Acked-by: Evgeniy Polyakov 

> ---
>  drivers/w1/masters/mxc_w1.c | 20 +---
>  1 file changed, 13 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/w1/masters/mxc_w1.c b/drivers/w1/masters/mxc_w1.c
> index 74f2e6e6202a..8851d441e5fd 100644
> --- a/drivers/w1/masters/mxc_w1.c
> +++ b/drivers/w1/masters/mxc_w1.c
> @@ -112,6 +112,10 @@ static int mxc_w1_probe(struct platform_device *pdev)
>  if (IS_ERR(mdev->clk))
>  return PTR_ERR(mdev->clk);
>
> + err = clk_prepare_enable(mdev->clk);
> + if (err)
> + return err;
> +
>  clkrate = clk_get_rate(mdev->clk);
>  if (clkrate < 1000)
>  dev_warn(>dev,
> @@ -125,12 +129,10 @@ static int mxc_w1_probe(struct platform_device *pdev)
>
>  res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
>  mdev->regs = devm_ioremap_resource(>dev, res);
> - if (IS_ERR(mdev->regs))
> - return PTR_ERR(mdev->regs);
> -
> - err = clk_prepare_enable(mdev->clk);
> - if (err)
> - return err;
> + if (IS_ERR(mdev->regs)) {
> + err = PTR_ERR(mdev->regs);
> + goto out_disable_clk;
> + }
>
>  /* Software reset 1-Wire module */
>  writeb(MXC_W1_RESET_RST, mdev->regs + MXC_W1_RESET);
> @@ -146,8 +148,12 @@ static int mxc_w1_probe(struct platform_device *pdev)
>
>  err = w1_add_master_device(>bus_master);
>  if (err)
> - clk_disable_unprepare(mdev->clk);
> + goto out_disable_clk;
>
> + return 0;
> +
> +out_disable_clk:
> + clk_disable_unprepare(mdev->clk);
>  return err;
>  }
>
> --
> 2.17.0

Re: [PATCH] 1wire: family module autoload fails because of upper/lower case mismatch.

[Evgeniy Polyakov]

Hi Ingo

Sorry for late reply and because of forcing you to resend the patch, your 
changes are correct

01.05.2018, 17:10, "Ingo Flaschberger" <ingo.flaschber...@gmail.com>:
> 1wire family module autoload fails because of upper/lower
>   case mismatch.
>
> Signed-off-by: Ingo Flaschberger <ingo.flaschber...@gmail.com>

Greg, please pull this patch into your tree.
Should it be stable material?

Acked-by: Evgeniy Polyakov <z...@ioremap.net>

> ---
>   drivers/w1/w1.c | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/w1/w1.c b/drivers/w1/w1.c
> index ab0931e..aa458f2 100644
> --- a/drivers/w1/w1.c
> +++ b/drivers/w1/w1.c
> @@ -741,7 +741,7 @@ int w1_attach_slave_device(struct w1_master *dev,
> struct w1_reg_num *rn)
>
>  /* slave modules need to be loaded in a context with unlocked
> mutex */
>  mutex_unlock(>mutex);
> -   request_module("w1-family-0x%02x", rn->family);
> +   request_module("w1-family-0x%02X", rn->family);
>  mutex_lock(>mutex);
>
>  spin_lock(_flock);
> --
> 2.7.4

Re: [PATCH] 1wire: family module autoload fails because of upper/lower case mismatch.

[Evgeniy Polyakov]

Hi Ingo

Sorry for late reply and because of forcing you to resend the patch, your 
changes are correct

01.05.2018, 17:10, "Ingo Flaschberger" :
> 1wire family module autoload fails because of upper/lower
>   case mismatch.
>
> Signed-off-by: Ingo Flaschberger 

Greg, please pull this patch into your tree.
Should it be stable material?

Acked-by: Evgeniy Polyakov 

> ---
>   drivers/w1/w1.c | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/w1/w1.c b/drivers/w1/w1.c
> index ab0931e..aa458f2 100644
> --- a/drivers/w1/w1.c
> +++ b/drivers/w1/w1.c
> @@ -741,7 +741,7 @@ int w1_attach_slave_device(struct w1_master *dev,
> struct w1_reg_num *rn)
>
>  /* slave modules need to be loaded in a context with unlocked
> mutex */
>  mutex_unlock(>mutex);
> -   request_module("w1-family-0x%02x", rn->family);
> +   request_module("w1-family-0x%02X", rn->family);
>  mutex_lock(>mutex);
>
>  spin_lock(_flock);
> --
> 2.7.4

Re: [PATCH] Enable clock before calling clk_get_rate() on it.

[Evgeniy Polyakov]

Hi Stefan

Nice catch, thank you!

19.04.2018, 16:02, "Stefan Potyra" <stefan.pot...@elektrobit.com>:
> According to the API, you may only call clk_get_rate() after actually
> enabling it.
>
> Found by Linux Driver Verification project (linuxtesting.org).
>
> Fixes: a5fd9139f74c w1: add 1-wire master driver for i.MX27 / i.MX31
> Signed-off-by: Stefan Potyra <stefan.pot...@elektrobit.com>

Acked-by: Evgeniy Polyakov <z...@ioremap.net>

Greg, please pull it into your tree. Is this a stable material?

> ---
>  drivers/w1/masters/mxc_w1.c | 8 
>  1 file changed, 4 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/w1/masters/mxc_w1.c b/drivers/w1/masters/mxc_w1.c
> index 74f2e6e6202a..a9599027d4ef 100644
> --- a/drivers/w1/masters/mxc_w1.c
> +++ b/drivers/w1/masters/mxc_w1.c
> @@ -112,6 +112,10 @@ static int mxc_w1_probe(struct platform_device *pdev)
>  if (IS_ERR(mdev->clk))
>  return PTR_ERR(mdev->clk);
>
> + err = clk_prepare_enable(mdev->clk);
> + if (err)
> + return err;
> +
>  clkrate = clk_get_rate(mdev->clk);
>  if (clkrate < 1000)
>  dev_warn(>dev,
> @@ -128,10 +132,6 @@ static int mxc_w1_probe(struct platform_device *pdev)
>  if (IS_ERR(mdev->regs))
>  return PTR_ERR(mdev->regs);
>
> - err = clk_prepare_enable(mdev->clk);
> - if (err)
> - return err;
> -
>  /* Software reset 1-Wire module */
>  writeb(MXC_W1_RESET_RST, mdev->regs + MXC_W1_RESET);
>  writeb(0, mdev->regs + MXC_W1_RESET);
> --
> 2.17.0

Re: [PATCH] Enable clock before calling clk_get_rate() on it.

[Evgeniy Polyakov]

Hi Stefan

Nice catch, thank you!

19.04.2018, 16:02, "Stefan Potyra" :
> According to the API, you may only call clk_get_rate() after actually
> enabling it.
>
> Found by Linux Driver Verification project (linuxtesting.org).
>
> Fixes: a5fd9139f74c w1: add 1-wire master driver for i.MX27 / i.MX31
> Signed-off-by: Stefan Potyra 

Acked-by: Evgeniy Polyakov 

Greg, please pull it into your tree. Is this a stable material?

> ---
>  drivers/w1/masters/mxc_w1.c | 8 
>  1 file changed, 4 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/w1/masters/mxc_w1.c b/drivers/w1/masters/mxc_w1.c
> index 74f2e6e6202a..a9599027d4ef 100644
> --- a/drivers/w1/masters/mxc_w1.c
> +++ b/drivers/w1/masters/mxc_w1.c
> @@ -112,6 +112,10 @@ static int mxc_w1_probe(struct platform_device *pdev)
>  if (IS_ERR(mdev->clk))
>  return PTR_ERR(mdev->clk);
>
> + err = clk_prepare_enable(mdev->clk);
> + if (err)
> + return err;
> +
>  clkrate = clk_get_rate(mdev->clk);
>  if (clkrate < 1000)
>  dev_warn(>dev,
> @@ -128,10 +132,6 @@ static int mxc_w1_probe(struct platform_device *pdev)
>  if (IS_ERR(mdev->regs))
>  return PTR_ERR(mdev->regs);
>
> - err = clk_prepare_enable(mdev->clk);
> - if (err)
> - return err;
> -
>  /* Software reset 1-Wire module */
>  writeb(MXC_W1_RESET_RST, mdev->regs + MXC_W1_RESET);
>  writeb(0, mdev->regs + MXC_W1_RESET);
> --
> 2.17.0

Re: [PATCH] connector: add parent pid and tgid to coredump and exit events

[Evgeniy Polyakov]

Stefan, hi

Sorry for delay.

26.04.2018, 15:04, "Stefan Strogin" <stefan.stro...@gmail.com>:
> Hi David, Evgeniy,
>
> Sorry to bother you, but could you please comment about the UAPI change and 
> the patch?

With 4-bytes pid_t everything looks fine, and I do not know arch where pid is 
larger currently, so it looks safe.

David, please pull it into your tree, or should it go via different path?

Acked-by: Evgeniy Polyakov <z...@ioremap.net>


>>  I don't see how it breaks UAPI. The point is that structures
>>  coredump_proc_event and exit_proc_event are members of *union*
>>  event_data, thus position of the existing data in the structure is
>>  unchanged. Furthermore, this change won't increase size of struct
>>  proc_event, because comm_proc_event (also a member of event_data) is
>>  of bigger size than the changed structures.
>>
>>  If I'm wrong, could you please explain what exactly will the change
>>  break in UAPI?
>>
>>  On 30/03/18 19:59, David Miller wrote:
>>>  From: Stefan Strogin <sstro...@cisco.com>
>>>  Date: Thu, 29 Mar 2018 17:12:47 +0300
>>>
>>>>  diff --git a/include/uapi/linux/cn_proc.h b/include/uapi/linux/cn_proc.h
>>>>  index 68ff25414700..db210625cee8 100644
>>>>  --- a/include/uapi/linux/cn_proc.h
>>>>  +++ b/include/uapi/linux/cn_proc.h
>>>>  @@ -116,12 +116,16 @@ struct proc_event {
>>>>   struct coredump_proc_event {
>>>>   __kernel_pid_t process_pid;
>>>>   __kernel_pid_t process_tgid;
>>>>  + __kernel_pid_t parent_pid;
>>>>  + __kernel_pid_t parent_tgid;
>>>>   } coredump;
>>>>
>>>>   struct exit_proc_event {
>>>>   __kernel_pid_t process_pid;
>>>>   __kernel_pid_t process_tgid;
>>>>   __u32 exit_code, exit_signal;
>>>>  + __kernel_pid_t parent_pid;
>>>>  + __kernel_pid_t parent_tgid;
>>>>   } exit;
>>>>
>>>>   } event_data;
>>>
>>>  I don't think you can add these members without breaking UAPI.

Re: [PATCH] connector: add parent pid and tgid to coredump and exit events

[Evgeniy Polyakov]

Stefan, hi

Sorry for delay.

26.04.2018, 15:04, "Stefan Strogin" :
> Hi David, Evgeniy,
>
> Sorry to bother you, but could you please comment about the UAPI change and 
> the patch?

With 4-bytes pid_t everything looks fine, and I do not know arch where pid is 
larger currently, so it looks safe.

David, please pull it into your tree, or should it go via different path?

Acked-by: Evgeniy Polyakov 


>>  I don't see how it breaks UAPI. The point is that structures
>>  coredump_proc_event and exit_proc_event are members of *union*
>>  event_data, thus position of the existing data in the structure is
>>  unchanged. Furthermore, this change won't increase size of struct
>>  proc_event, because comm_proc_event (also a member of event_data) is
>>  of bigger size than the changed structures.
>>
>>  If I'm wrong, could you please explain what exactly will the change
>>  break in UAPI?
>>
>>  On 30/03/18 19:59, David Miller wrote:
>>>  From: Stefan Strogin 
>>>  Date: Thu, 29 Mar 2018 17:12:47 +0300
>>>
>>>>  diff --git a/include/uapi/linux/cn_proc.h b/include/uapi/linux/cn_proc.h
>>>>  index 68ff25414700..db210625cee8 100644
>>>>  --- a/include/uapi/linux/cn_proc.h
>>>>  +++ b/include/uapi/linux/cn_proc.h
>>>>  @@ -116,12 +116,16 @@ struct proc_event {
>>>>   struct coredump_proc_event {
>>>>   __kernel_pid_t process_pid;
>>>>   __kernel_pid_t process_tgid;
>>>>  + __kernel_pid_t parent_pid;
>>>>  + __kernel_pid_t parent_tgid;
>>>>   } coredump;
>>>>
>>>>   struct exit_proc_event {
>>>>   __kernel_pid_t process_pid;
>>>>   __kernel_pid_t process_tgid;
>>>>   __u32 exit_code, exit_signal;
>>>>  + __kernel_pid_t parent_pid;
>>>>  + __kernel_pid_t parent_tgid;
>>>>   } exit;
>>>>
>>>>   } event_data;
>>>
>>>  I don't think you can add these members without breaking UAPI.

Re: [PATCH] Fixed typo in onewire generic doc

[Evgeniy Polyakov]

Hi everyone

I'm really sorry for that long delay.

Was this patch accepted or should I push it upstream?

20.12.2017, 22:09, "Gergo Huszty" :
> Onewire devices has 6 byte long unique serial numbers, 1 byte family
> code and 1 byte CRC. Linux sysfs presents the device folder in the
> form of familyID-deviceID, so CRC is not shown. The consequence is
> that the device serial number is always a 12 long hex-string, but
> doc says 13 in one place. This is corrected by this change.
> Reference: https://en.wikipedia.org/wiki/1-Wire
>
> Signed-off-by: Gergo Huszty 
> ---
>  Documentation/w1/w1.generic | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/Documentation/w1/w1.generic b/Documentation/w1/w1.generic
> index b3ffaf8cfab2..c51b1ab012d0 100644
> --- a/Documentation/w1/w1.generic
> +++ b/Documentation/w1/w1.generic
> @@ -76,7 +76,7 @@ See struct w1_bus_master definition in w1.h for details.
>
>  w1 master sysfs interface
>  --
> - - A directory for a found device. The format is 
> family-serial
> + - A directory for a found device. The format is 
> family-serial
>  bus - (standard) symlink to the w1 bus
>  driver - (standard) symlink to the w1 driver
>  w1_master_add - (rw) manually register a slave device
> --
> 2.15.1

Re: [PATCH] Fixed typo in onewire generic doc

[Evgeniy Polyakov]

Hi everyone

I'm really sorry for that long delay.

Was this patch accepted or should I push it upstream?

20.12.2017, 22:09, "Gergo Huszty" :
> Onewire devices has 6 byte long unique serial numbers, 1 byte family
> code and 1 byte CRC. Linux sysfs presents the device folder in the
> form of familyID-deviceID, so CRC is not shown. The consequence is
> that the device serial number is always a 12 long hex-string, but
> doc says 13 in one place. This is corrected by this change.
> Reference: https://en.wikipedia.org/wiki/1-Wire
>
> Signed-off-by: Gergo Huszty 
> ---
>  Documentation/w1/w1.generic | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/Documentation/w1/w1.generic b/Documentation/w1/w1.generic
> index b3ffaf8cfab2..c51b1ab012d0 100644
> --- a/Documentation/w1/w1.generic
> +++ b/Documentation/w1/w1.generic
> @@ -76,7 +76,7 @@ See struct w1_bus_master definition in w1.h for details.
>
>  w1 master sysfs interface
>  --
> - - A directory for a found device. The format is 
> family-serial
> + - A directory for a found device. The format is 
> family-serial
>  bus - (standard) symlink to the w1 bus
>  driver - (standard) symlink to the w1 driver
>  w1_master_add - (rw) manually register a slave device
> --
> 2.15.1

Re: [PATCH] w1: w1-gpio: Convert to use GPIO descriptors

[Evgeniy Polyakov]

Hi Linus

Sorry for late reply

20.11.2017, 11:47, "Linus Walleij" :
> The w1 master driver includes a complete open drain emulation
> reimplementation among other things.
>
> This converts the driver and all board files using it to use
> GPIO descriptors associated with the device to look up the
> GPIO wire, as well ass the optional pull-up GPIO line.

I'm not familiar with gpio platform drivers, but these changes looks good.
Did it find its way into the tree yet?

If not, please add appropriate maintainers, I've added Ville Syrjala to copy.

> When probed from the device tree, the driver will just pick
> descriptors and use them right off. For the two board files
> in the kernel, we add descriptor lookups so we do not need
> to keep any old platform data handling around for the GPIO
> lines.
>
> As the platform data is also a state container for this driver,
> we augment it to contain the GPIO descriptors.
>
> w1_gpio_write_bit_dir() and w1_gpio_write_bit_val() are gone
> since this pair was a reimplementation of open drain emulation
> which is now handled by gpiolib.
>
> The special "linux,open-drain" flag is a bit of mishap here:
> it has the same semantic as the same flags in I2C: it means
> that something in the platform is setting up the line as
> open drain behind our back. We handle this the same way as
> in I2C.
>
> To drive the pull-up, we need to bypass open drain emulation
> in gpiolib for the line, and this is done by driving it high
> using gpiod_set_raw_value() which has been augmented to have
> the semantic of overriding the open drain emulation.
>
> We also augment the documentation to reflect the way to pass
> GPIO descriptors from the machine.
>
> Signed-off-by: Linus Walleij 
> ---
> I don't have a W1-capable system myself so testing would be
> appreciated. This needs to be applied to Torvald's HEAD
> or v4.15-rc1 when it is tagged to work, as it is using the new
> infrastructure updates I merged in the v4.15 merge window.
> ---
>  Documentation/w1/masters/w1-gpio | 17 +++-
>  arch/arm/mach-ixp4xx/vulcan-setup.c | 13 +++-
>  arch/arm/mach-pxa/raumfeld.c | 16 ++--
>  drivers/w1/masters/w1-gpio.c | 149 +++-
>  include/linux/w1-gpio.h | 9 +--
>  5 files changed, 101 insertions(+), 103 deletions(-)
>
> diff --git a/Documentation/w1/masters/w1-gpio 
> b/Documentation/w1/masters/w1-gpio
> index af5d3b4aa851..623961d9e83f 100644
> --- a/Documentation/w1/masters/w1-gpio
> +++ b/Documentation/w1/masters/w1-gpio
> @@ -8,17 +8,27 @@ Description
>  ---
>
>  GPIO 1-wire bus master driver. The driver uses the GPIO API to control the
> -wire and the GPIO pin can be specified using platform data.
> +wire and the GPIO pin can be specified using GPIO machine descriptor tables.
> +It is also possible to define the master using device tree, see
> +Documentation/devicetree/bindings/w1/w1-gpio.txt
>
>  Example (mach-at91)
>  ---
>
> +#include 
>  #include 
>
> +static struct gpiod_lookup_table foo_w1_gpiod_table = {
> + .dev_id = "w1-gpio",
> + .table = {
> + GPIO_LOOKUP_IDX("at91-gpio", AT91_PIN_PB20, NULL, 0,
> + GPIO_ACTIVE_HIGH|GPIO_OPEN_DRAIN),
> + },
> +};
> +
>  static struct w1_gpio_platform_data foo_w1_gpio_pdata = {
> - .pin = AT91_PIN_PB20,
> - .is_open_drain = 1,
> + .ext_pullup_enable_pin = -EINVAL,
>  };
>
>  static struct platform_device foo_w1_device = {
> @@ -30,4 +40,5 @@ static struct platform_device foo_w1_device = {
>  ...
>  at91_set_GPIO_periph(foo_w1_gpio_pdata.pin, 1);
>  at91_set_multi_drive(foo_w1_gpio_pdata.pin, 1);
> + gpiod_add_lookup_table(_w1_gpiod_table);
>  platform_device_register(_w1_device);
> diff --git a/arch/arm/mach-ixp4xx/vulcan-setup.c 
> b/arch/arm/mach-ixp4xx/vulcan-setup.c
> index 731fb2019ecb..2c03d2f6b647 100644
> --- a/arch/arm/mach-ixp4xx/vulcan-setup.c
> +++ b/arch/arm/mach-ixp4xx/vulcan-setup.c
> @@ -16,6 +16,7 @@
>  #include 
>  #include 
>  #include 
> +#include 
>  #include 
>  #include 
>  #include 
> @@ -162,9 +163,16 @@ static struct platform_device vulcan_max6369 = {
>  .num_resources = 1,
>  };
>
> +static struct gpiod_lookup_table vulcan_w1_gpiod_table = {
> + .dev_id = "w1-gpio",
> + .table = {
> + GPIO_LOOKUP_IDX("IXP4XX_GPIO_CHIP", 14, NULL, 0,
> + GPIO_ACTIVE_HIGH | GPIO_OPEN_DRAIN),
> + },
> +};
> +
>  static struct w1_gpio_platform_data vulcan_w1_gpio_pdata = {
> - .pin = 14,
> - .ext_pullup_enable_pin = -EINVAL,
> + /* Intentionally left blank */
>  };
>
>  static struct platform_device vulcan_w1_gpio = {
> @@ -233,6 +241,7 @@ static void __init vulcan_init(void)
>    IXP4XX_EXP_BUS_WR_EN |
>    IXP4XX_EXP_BUS_BYTE_EN;
>
> + gpiod_add_lookup_table(_w1_gpiod_table);
>  platform_add_devices(vulcan_devices, ARRAY_SIZE(vulcan_devices));
>  }
>
> diff --git a/arch/arm/mach-pxa/raumfeld.c b/arch/arm/mach-pxa/raumfeld.c
> index 9d662fed03ec..feddca7f3540 

Re: [PATCH] w1: w1-gpio: Convert to use GPIO descriptors

[Evgeniy Polyakov]

Hi Linus

Sorry for late reply

20.11.2017, 11:47, "Linus Walleij" :
> The w1 master driver includes a complete open drain emulation
> reimplementation among other things.
>
> This converts the driver and all board files using it to use
> GPIO descriptors associated with the device to look up the
> GPIO wire, as well ass the optional pull-up GPIO line.

I'm not familiar with gpio platform drivers, but these changes looks good.
Did it find its way into the tree yet?

If not, please add appropriate maintainers, I've added Ville Syrjala to copy.

> When probed from the device tree, the driver will just pick
> descriptors and use them right off. For the two board files
> in the kernel, we add descriptor lookups so we do not need
> to keep any old platform data handling around for the GPIO
> lines.
>
> As the platform data is also a state container for this driver,
> we augment it to contain the GPIO descriptors.
>
> w1_gpio_write_bit_dir() and w1_gpio_write_bit_val() are gone
> since this pair was a reimplementation of open drain emulation
> which is now handled by gpiolib.
>
> The special "linux,open-drain" flag is a bit of mishap here:
> it has the same semantic as the same flags in I2C: it means
> that something in the platform is setting up the line as
> open drain behind our back. We handle this the same way as
> in I2C.
>
> To drive the pull-up, we need to bypass open drain emulation
> in gpiolib for the line, and this is done by driving it high
> using gpiod_set_raw_value() which has been augmented to have
> the semantic of overriding the open drain emulation.
>
> We also augment the documentation to reflect the way to pass
> GPIO descriptors from the machine.
>
> Signed-off-by: Linus Walleij 
> ---
> I don't have a W1-capable system myself so testing would be
> appreciated. This needs to be applied to Torvald's HEAD
> or v4.15-rc1 when it is tagged to work, as it is using the new
> infrastructure updates I merged in the v4.15 merge window.
> ---
>  Documentation/w1/masters/w1-gpio | 17 +++-
>  arch/arm/mach-ixp4xx/vulcan-setup.c | 13 +++-
>  arch/arm/mach-pxa/raumfeld.c | 16 ++--
>  drivers/w1/masters/w1-gpio.c | 149 +++-
>  include/linux/w1-gpio.h | 9 +--
>  5 files changed, 101 insertions(+), 103 deletions(-)
>
> diff --git a/Documentation/w1/masters/w1-gpio 
> b/Documentation/w1/masters/w1-gpio
> index af5d3b4aa851..623961d9e83f 100644
> --- a/Documentation/w1/masters/w1-gpio
> +++ b/Documentation/w1/masters/w1-gpio
> @@ -8,17 +8,27 @@ Description
>  ---
>
>  GPIO 1-wire bus master driver. The driver uses the GPIO API to control the
> -wire and the GPIO pin can be specified using platform data.
> +wire and the GPIO pin can be specified using GPIO machine descriptor tables.
> +It is also possible to define the master using device tree, see
> +Documentation/devicetree/bindings/w1/w1-gpio.txt
>
>  Example (mach-at91)
>  ---
>
> +#include 
>  #include 
>
> +static struct gpiod_lookup_table foo_w1_gpiod_table = {
> + .dev_id = "w1-gpio",
> + .table = {
> + GPIO_LOOKUP_IDX("at91-gpio", AT91_PIN_PB20, NULL, 0,
> + GPIO_ACTIVE_HIGH|GPIO_OPEN_DRAIN),
> + },
> +};
> +
>  static struct w1_gpio_platform_data foo_w1_gpio_pdata = {
> - .pin = AT91_PIN_PB20,
> - .is_open_drain = 1,
> + .ext_pullup_enable_pin = -EINVAL,
>  };
>
>  static struct platform_device foo_w1_device = {
> @@ -30,4 +40,5 @@ static struct platform_device foo_w1_device = {
>  ...
>  at91_set_GPIO_periph(foo_w1_gpio_pdata.pin, 1);
>  at91_set_multi_drive(foo_w1_gpio_pdata.pin, 1);
> + gpiod_add_lookup_table(_w1_gpiod_table);
>  platform_device_register(_w1_device);
> diff --git a/arch/arm/mach-ixp4xx/vulcan-setup.c 
> b/arch/arm/mach-ixp4xx/vulcan-setup.c
> index 731fb2019ecb..2c03d2f6b647 100644
> --- a/arch/arm/mach-ixp4xx/vulcan-setup.c
> +++ b/arch/arm/mach-ixp4xx/vulcan-setup.c
> @@ -16,6 +16,7 @@
>  #include 
>  #include 
>  #include 
> +#include 
>  #include 
>  #include 
>  #include 
> @@ -162,9 +163,16 @@ static struct platform_device vulcan_max6369 = {
>  .num_resources = 1,
>  };
>
> +static struct gpiod_lookup_table vulcan_w1_gpiod_table = {
> + .dev_id = "w1-gpio",
> + .table = {
> + GPIO_LOOKUP_IDX("IXP4XX_GPIO_CHIP", 14, NULL, 0,
> + GPIO_ACTIVE_HIGH | GPIO_OPEN_DRAIN),
> + },
> +};
> +
>  static struct w1_gpio_platform_data vulcan_w1_gpio_pdata = {
> - .pin = 14,
> - .ext_pullup_enable_pin = -EINVAL,
> + /* Intentionally left blank */
>  };
>
>  static struct platform_device vulcan_w1_gpio = {
> @@ -233,6 +241,7 @@ static void __init vulcan_init(void)
>    IXP4XX_EXP_BUS_WR_EN |
>    IXP4XX_EXP_BUS_BYTE_EN;
>
> + gpiod_add_lookup_table(_w1_gpiod_table);
>  platform_add_devices(vulcan_devices, ARRAY_SIZE(vulcan_devices));
>  }
>
> diff --git a/arch/arm/mach-pxa/raumfeld.c b/arch/arm/mach-pxa/raumfeld.c
> index 9d662fed03ec..feddca7f3540 100644
> --- a/arch/arm/mach-pxa/raumfeld.c
> +++ 

Re: [PATCH] w1: remove redundant assignments to search_bit and last_rn

[Evgeniy Polyakov]

Hi

07.11.2017, 21:49, "Colin King" <colin.k...@canonical.com>:
> From: Colin Ian King <colin.k...@canonical.com>
>
> Variables search_bit and last_rn are assigned values before a while-loop
> however these initial values are never read (as they are overwritten
> inside the loop). Thus these initial assignments are redundant and can
> be removed. Cleans up clang warnings:
>
> drivers/w1/w1.c:967:2: warning: Value stored to 'search_bit' is never
> read
> drivers/w1/w1.c:969:2: warning: Value stored to 'last_rn' is never read

This looks good, thank you.
Please pull it into janitors tree if you haven't yet.

> Signed-off-by: Colin Ian King <colin.k...@canonical.com>

Acked-by: Evgeniy Polyakov <z...@ioremap.net>

> ---
>  drivers/w1/w1.c | 2 --
>  1 file changed, 2 deletions(-)
>
> diff --git a/drivers/w1/w1.c b/drivers/w1/w1.c
> index 0c2a5a8327bd..c29f6c5dda3c 100644
> --- a/drivers/w1/w1.c
> +++ b/drivers/w1/w1.c
> @@ -964,9 +964,7 @@ void w1_search(struct w1_master *dev, u8 search_type, 
> w1_slave_found_callback cb
>  int search_bit, desc_bit;
>  u8 triplet_ret = 0;
>
> - search_bit = 0;
>  rn = dev->search_id;
> - last_rn = 0;
>  last_device = 0;
>  last_zero = -1;
>
> --
> 2.14.1

Re: [PATCH] w1: remove redundant assignments to search_bit and last_rn

[Evgeniy Polyakov]

Hi

07.11.2017, 21:49, "Colin King" :
> From: Colin Ian King 
>
> Variables search_bit and last_rn are assigned values before a while-loop
> however these initial values are never read (as they are overwritten
> inside the loop). Thus these initial assignments are redundant and can
> be removed. Cleans up clang warnings:
>
> drivers/w1/w1.c:967:2: warning: Value stored to 'search_bit' is never
> read
> drivers/w1/w1.c:969:2: warning: Value stored to 'last_rn' is never read

This looks good, thank you.
Please pull it into janitors tree if you haven't yet.

> Signed-off-by: Colin Ian King 

Acked-by: Evgeniy Polyakov 

> ---
>  drivers/w1/w1.c | 2 --
>  1 file changed, 2 deletions(-)
>
> diff --git a/drivers/w1/w1.c b/drivers/w1/w1.c
> index 0c2a5a8327bd..c29f6c5dda3c 100644
> --- a/drivers/w1/w1.c
> +++ b/drivers/w1/w1.c
> @@ -964,9 +964,7 @@ void w1_search(struct w1_master *dev, u8 search_type, 
> w1_slave_found_callback cb
>  int search_bit, desc_bit;
>  u8 triplet_ret = 0;
>
> - search_bit = 0;
>  rn = dev->search_id;
> - last_rn = 0;
>  last_device = 0;
>  last_zero = -1;
>
> --
> 2.14.1

Re: [PATCH v2] w1: driver for serial linkUSB, linkOEM, LINK devices

[Evgeniy Polyakov]

Hi

Sorry for delay

13.11.2017, 00:05, "Hans-Frieder Vogt" <hfv...@gmx.net>:
> attached is the revised version of a driver that supports the serial OneWire 
> masters from iButtonLink(TM) in the w1 kernel driver. In order to be usable 
> it needs an updated userland tool (patch included in documentation file). The 
> kernel patch is against linux-next. Please try and comment.
>
> v2 of the patch set changes the following:
> - based on review input from Evgeniy (thanks!), introduced a helper function 
> to re-use more code and generally improved the style of the code (couldn't 
> get rid of all problems checkpatch identified, but many of them)
> - introduced a version check to make sure the hardware belongs to the 
> supported devices
> - removed a dependency on little endian system
> - further testing revealed re-entry problems. The protection of critical code 
> areas has therefore been changed from local spinlocks to a more toplevel 
> mutex-based system to enforce serialisation. I have not run into any problems 
> with unexpected feedback from the link device since.

This looks good to me, thank you!
Greg, please pull it into your tree

> Signed-off by: Hans-Frieder Vogt <hfv...@gmx.net>

Acked-by: Evgeniy Polyakov <z...@ioremap.net>

>  Documentation/w1/masters/00-INDEX   |    2
>  Documentation/w1/masters/linkserial |   44 ++
>  drivers/w1/masters/Kconfig  |   10
>  drivers/w1/masters/Makefile |    1
>  drivers/w1/masters/linkserial.c |  538 
> 
>  include/uapi/linux/serio.h  |    1
>  6 files changed, 596 insertions(+)
>
> diff --git a/Documentation/w1/masters/00-INDEX 
> b/Documentation/w1/masters/00-INDEX
> index 8330cf9325f0..33c522854126 100644
> --- a/Documentation/w1/masters/00-INDEX
> +++ b/Documentation/w1/masters/00-INDEX
> @@ -4,6 +4,8 @@ ds2482
>  - The Maxim/Dallas Semiconductor DS2482 provides 1-wire busses.
>  ds2490
>  - The Maxim/Dallas Semiconductor DS2490 builds USB <-> W1 bridges.
> +linkserial
> + - Serial to W1 bridge provided by iButtonLink devices.
>  mxc-w1
>  - W1 master controller driver found on Freescale MX2/MX3 SoCs
>  omap-hdq
> diff --git a/Documentation/w1/masters/linkserial 
> b/Documentation/w1/masters/linkserial
> new file mode 100644
> index ..d8b0a209ce4a
> --- /dev/null
> +++ b/Documentation/w1/masters/linkserial
> @@ -0,0 +1,44 @@
> +Kernel driver linkserial
> +
> +
> +Supported devices
> + * LinkUSB(TM)
> + * LinkOEM(TM)
> + * LINK(TM) untested
> +
> +Author: Hans-Frieder Vogt <hfv...@gmx.net>
> +
> +
> +Description
> +---
> +
> +1-wire bus master driver for iButtonLink LLC devices. These devices can 
> operate
> +in 2 modes: DS9097U Emulation and ASCII mode. The driver linkserial uses the
> +ASCII command interface only.
> +
> +This driver needs an adapted userspace program inputattach to be used. The
> +following patch modifies inputattach as needed:
> +
> +--- a/inputattach.c 2016-08-09 13:04:05.0 +0200
>  b/inputattach.c 2017-10-14 23:49:08.594165130 +0200
> +@@ -49,6 +49,9 @@
> + #ifdef SYSTEMD_SUPPORT
> + #include 
> + #endif
> ++#ifndef SERIO_ILINK
> ++#define SERIO_ILINK 0x42
> ++#endif
> +
> + static int readchar(int fd, unsigned char *c, int timeout)
> + {
> +@@ -867,6 +870,9 @@ static struct input_types input_types[]
> + { "--pulse8-cec", "-pulse8-cec", "Pulse Eight HDMI CEC dongle",
> + B9600, CS8,
> + SERIO_PULSE8_CEC, 0x00, 0x00, 0, NULL },
> ++{ "--linkserial", "-linkserial", "Link Onewire master",
> ++ B9600, CS8,
> ++ SERIO_ILINK, 0x00, 0x00, 0, NULL },
> + { NULL, NULL, NULL, 0, 0, 0, 0, 0, 0, NULL }
> + };
> +
> +
> diff --git a/drivers/w1/masters/Kconfig b/drivers/w1/masters/Kconfig
> index 1708b2300c7a..eba57c2f0751 100644
> --- a/drivers/w1/masters/Kconfig
> +++ b/drivers/w1/masters/Kconfig
> @@ -34,6 +34,16 @@ config W1_MASTER_DS2482
>    This driver can also be built as a module. If so, the module
>    will be called ds2482.
>
> +config W1_MASTER_ILINK
> + tristate "iButtonLink(TM) serial to 1-Wire bridge"
> + depends on SERIO
> + help
> + Say Y here to get support for the iButtonLink(TM) serial to 1-Wire
> + bridges like the LINK(TM), the LinkUSB(TM) and the LinkOEM(TM).
> +
> + This driver can also be built as a module. If so, the module
> + will be called linkserial.
> +
>  config W1_MASTER_MXC
>  tristate "Freescale MXC 1-wire busmaster"
>  depends on ARCH_MXC || COMPILE_TEST
> diff --git a/drivers/w1/ma

Re: [PATCH v2] w1: driver for serial linkUSB, linkOEM, LINK devices

[Evgeniy Polyakov]

Hi

Sorry for delay

13.11.2017, 00:05, "Hans-Frieder Vogt" :
> attached is the revised version of a driver that supports the serial OneWire 
> masters from iButtonLink(TM) in the w1 kernel driver. In order to be usable 
> it needs an updated userland tool (patch included in documentation file). The 
> kernel patch is against linux-next. Please try and comment.
>
> v2 of the patch set changes the following:
> - based on review input from Evgeniy (thanks!), introduced a helper function 
> to re-use more code and generally improved the style of the code (couldn't 
> get rid of all problems checkpatch identified, but many of them)
> - introduced a version check to make sure the hardware belongs to the 
> supported devices
> - removed a dependency on little endian system
> - further testing revealed re-entry problems. The protection of critical code 
> areas has therefore been changed from local spinlocks to a more toplevel 
> mutex-based system to enforce serialisation. I have not run into any problems 
> with unexpected feedback from the link device since.

This looks good to me, thank you!
Greg, please pull it into your tree

> Signed-off by: Hans-Frieder Vogt 

Acked-by: Evgeniy Polyakov 

>  Documentation/w1/masters/00-INDEX   |    2
>  Documentation/w1/masters/linkserial |   44 ++
>  drivers/w1/masters/Kconfig  |   10
>  drivers/w1/masters/Makefile |    1
>  drivers/w1/masters/linkserial.c |  538 
> 
>  include/uapi/linux/serio.h  |    1
>  6 files changed, 596 insertions(+)
>
> diff --git a/Documentation/w1/masters/00-INDEX 
> b/Documentation/w1/masters/00-INDEX
> index 8330cf9325f0..33c522854126 100644
> --- a/Documentation/w1/masters/00-INDEX
> +++ b/Documentation/w1/masters/00-INDEX
> @@ -4,6 +4,8 @@ ds2482
>  - The Maxim/Dallas Semiconductor DS2482 provides 1-wire busses.
>  ds2490
>  - The Maxim/Dallas Semiconductor DS2490 builds USB <-> W1 bridges.
> +linkserial
> + - Serial to W1 bridge provided by iButtonLink devices.
>  mxc-w1
>  - W1 master controller driver found on Freescale MX2/MX3 SoCs
>  omap-hdq
> diff --git a/Documentation/w1/masters/linkserial 
> b/Documentation/w1/masters/linkserial
> new file mode 100644
> index ..d8b0a209ce4a
> --- /dev/null
> +++ b/Documentation/w1/masters/linkserial
> @@ -0,0 +1,44 @@
> +Kernel driver linkserial
> +
> +
> +Supported devices
> + * LinkUSB(TM)
> + * LinkOEM(TM)
> + * LINK(TM) untested
> +
> +Author: Hans-Frieder Vogt 
> +
> +
> +Description
> +---
> +
> +1-wire bus master driver for iButtonLink LLC devices. These devices can 
> operate
> +in 2 modes: DS9097U Emulation and ASCII mode. The driver linkserial uses the
> +ASCII command interface only.
> +
> +This driver needs an adapted userspace program inputattach to be used. The
> +following patch modifies inputattach as needed:
> +
> +--- a/inputattach.c 2016-08-09 13:04:05.0 +0200
>  b/inputattach.c 2017-10-14 23:49:08.594165130 +0200
> +@@ -49,6 +49,9 @@
> + #ifdef SYSTEMD_SUPPORT
> + #include 
> + #endif
> ++#ifndef SERIO_ILINK
> ++#define SERIO_ILINK 0x42
> ++#endif
> +
> + static int readchar(int fd, unsigned char *c, int timeout)
> + {
> +@@ -867,6 +870,9 @@ static struct input_types input_types[]
> + { "--pulse8-cec", "-pulse8-cec", "Pulse Eight HDMI CEC dongle",
> + B9600, CS8,
> + SERIO_PULSE8_CEC, 0x00, 0x00, 0, NULL },
> ++{ "--linkserial", "-linkserial", "Link Onewire master",
> ++ B9600, CS8,
> ++ SERIO_ILINK, 0x00, 0x00, 0, NULL },
> + { NULL, NULL, NULL, 0, 0, 0, 0, 0, 0, NULL }
> + };
> +
> +
> diff --git a/drivers/w1/masters/Kconfig b/drivers/w1/masters/Kconfig
> index 1708b2300c7a..eba57c2f0751 100644
> --- a/drivers/w1/masters/Kconfig
> +++ b/drivers/w1/masters/Kconfig
> @@ -34,6 +34,16 @@ config W1_MASTER_DS2482
>    This driver can also be built as a module. If so, the module
>    will be called ds2482.
>
> +config W1_MASTER_ILINK
> + tristate "iButtonLink(TM) serial to 1-Wire bridge"
> + depends on SERIO
> + help
> + Say Y here to get support for the iButtonLink(TM) serial to 1-Wire
> + bridges like the LINK(TM), the LinkUSB(TM) and the LinkOEM(TM).
> +
> + This driver can also be built as a module. If so, the module
> + will be called linkserial.
> +
>  config W1_MASTER_MXC
>  tristate "Freescale MXC 1-wire busmaster"
>  depends on ARCH_MXC || COMPILE_TEST
> diff --git a/drivers/w1/masters/Makefile b/drivers/w1/masters/Makefile
> index 18954cae4256..3fe656187c17 10

Re: [PATCH] w1 driver for serial linkUSB, linkOEM, LINK devices

[Evgeniy Polyakov]

Hi

17.10.2017, 23:01, "Hans-Frieder Vogt" :
> attached is a driver that supports the serial OneWire masters from 
> iButtonLink(TM) in the w1 kernel driver. In order to be usable it needs an 
> updated userland tool (patch included in documentation file). The patch is 
> against linux-next. Please try and comment.

This looks good from w1 point of view except minor nits, but I know nothing 
about serio links, so it might worth asking appropriate parties

> +static int link_mode_ascii(void *data)

You always have link_data when calling this function, no need to dereference 
void pointer all the time

> +static int link_mode_hex(void *data)

Same here

> +static void link_w1_write_byte(void *data, u8 byte)
> +{
> + struct link_data *link = data;
> + struct serio *serio = link->serio;
> + int len;
> + unsigned long fl;
> +
> + /* make sure we are in hex mode */
> + link_mode_hex(data);
> +
> + spin_lock_irqsave(>lock, fl);
> + serio_write(serio, val2char[byte >> 4]);
> + serio_write(serio, val2char[byte & 0x0f]);
> + link->pos = 0;
> + link->state = LINK_STATE_BUSY;
> + link->expected = 2;
> + spin_unlock_irqrestore(>lock, fl);

This pattern (with 2 different control characters) repeats many times across 
the code, should it live in its own helper function?

> +static void link_w1_write_block(void *data, const u8 *buf, int len)
> +{
> + struct link_data *link = data;
> + struct serio *serio = link->serio;
> + int i, l;
> + const u8 *p = buf;
> + unsigned long fl;
> +
> + if (len <= 0)
> + return;
> + if (len*2 > BUFFER_SIZE)
> + return;
> +
> + /* make sure we are in hex mode */
> + link_mode_hex(data);
> +
> + spin_lock_irqsave(>lock, fl);
> + for (i=0; i + serio_write(serio, val2char[*p >> 4]);
> + serio_write(serio, val2char[*p & 0x0f]);
> + }
> + link->pos = 0;
> + link->state = LINK_STATE_BUSY;
> + link->expected = 2*len;
> + spin_unlock_irqrestore(>lock, fl);

Is this 100% overflow-free?
Since serio interrupt checks whether the number of consumed equals to expected 
field and writes 0 past the buffer...

> +static u8 link_w1_set_pullup(void *data, int delay)
> +{
> + struct link_data *link = data;
> + int result = 0;
> + unsigned long fl;
> +
> + printk(KERN_INFO "pullup: %d\n", delay);

Please drop this print

> +static irqreturn_t linkserial_interrupt(struct serio *serio, unsigned char 
> data,
> + unsigned int flags)
> +{
> + struct link_data *link = serio_get_drvdata(serio);
> +
> + link->buffer[link->pos++] = data;
> +
> + switch (link->mode) {
> + case MODE_HEX:
> + if (link->pos >= link->expected) {
> + link->buffer[link->pos] = '\0';

I meant this write in my comment in link_w1_write_block()

Re: [PATCH] w1 driver for serial linkUSB, linkOEM, LINK devices

[Evgeniy Polyakov]

Hi

17.10.2017, 23:01, "Hans-Frieder Vogt" :
> attached is a driver that supports the serial OneWire masters from 
> iButtonLink(TM) in the w1 kernel driver. In order to be usable it needs an 
> updated userland tool (patch included in documentation file). The patch is 
> against linux-next. Please try and comment.

This looks good from w1 point of view except minor nits, but I know nothing 
about serio links, so it might worth asking appropriate parties

> +static int link_mode_ascii(void *data)

You always have link_data when calling this function, no need to dereference 
void pointer all the time

> +static int link_mode_hex(void *data)

Same here

> +static void link_w1_write_byte(void *data, u8 byte)
> +{
> + struct link_data *link = data;
> + struct serio *serio = link->serio;
> + int len;
> + unsigned long fl;
> +
> + /* make sure we are in hex mode */
> + link_mode_hex(data);
> +
> + spin_lock_irqsave(>lock, fl);
> + serio_write(serio, val2char[byte >> 4]);
> + serio_write(serio, val2char[byte & 0x0f]);
> + link->pos = 0;
> + link->state = LINK_STATE_BUSY;
> + link->expected = 2;
> + spin_unlock_irqrestore(>lock, fl);

This pattern (with 2 different control characters) repeats many times across 
the code, should it live in its own helper function?

> +static void link_w1_write_block(void *data, const u8 *buf, int len)
> +{
> + struct link_data *link = data;
> + struct serio *serio = link->serio;
> + int i, l;
> + const u8 *p = buf;
> + unsigned long fl;
> +
> + if (len <= 0)
> + return;
> + if (len*2 > BUFFER_SIZE)
> + return;
> +
> + /* make sure we are in hex mode */
> + link_mode_hex(data);
> +
> + spin_lock_irqsave(>lock, fl);
> + for (i=0; i + serio_write(serio, val2char[*p >> 4]);
> + serio_write(serio, val2char[*p & 0x0f]);
> + }
> + link->pos = 0;
> + link->state = LINK_STATE_BUSY;
> + link->expected = 2*len;
> + spin_unlock_irqrestore(>lock, fl);

Is this 100% overflow-free?
Since serio interrupt checks whether the number of consumed equals to expected 
field and writes 0 past the buffer...

> +static u8 link_w1_set_pullup(void *data, int delay)
> +{
> + struct link_data *link = data;
> + int result = 0;
> + unsigned long fl;
> +
> + printk(KERN_INFO "pullup: %d\n", delay);

Please drop this print

> +static irqreturn_t linkserial_interrupt(struct serio *serio, unsigned char 
> data,
> + unsigned int flags)
> +{
> + struct link_data *link = serio_get_drvdata(serio);
> +
> + link->buffer[link->pos++] = data;
> +
> + switch (link->mode) {
> + case MODE_HEX:
> + if (link->pos >= link->expected) {
> + link->buffer[link->pos] = '\0';

I meant this write in my comment in link_w1_write_block()

Re: [PATCH] w1: keep balance of mutex locks and refcnts

[Evgeniy Polyakov]

Hi Alexey

07.10.2017, 20:59, "Alexey Khoroshilov" <khoroshi...@ispras.ru>:
> Is it possible to switch to a nested variant:
> mutex_lock-atomic_inc-atomic_dec-mutex_unlock
> or
> atomic_inc-mutex_lock-mutex_unlock-atomic_dec
> ?

Yeah, you are right, it is a bit messy - we drop the lock while sleeping 
waiting for the bus master to complete operation,
and during this period family driver has to be referenced.

But we can easily grab the reference earlier and then try to lock the bus, so 
the second variant will work.

> --
> Alexey
>
> On 01.10.2017 08:55, Evgeniy Polyakov wrote:
>>  Hi Alex
>>
>>  29.09.2017, 23:23, "Alexey Khoroshilov" <khoroshi...@ispras.ru>:
>>>  w1_therm_eeprom() and w1_DS18B20_precision() decrement THERM_REFCNT
>>>  on error paths, while they did not increment it yet.
>>>
>>>  read_therm() unlocks bus mutex on some error paths,
>>>  while it is not acquired.
>>>
>>>  The patch makes sure all the functions keep the balance in usage of
>>>  the mutex and the THERM_REFCNT.
>>>
>>>  Found by Linux Driver Verification project (linuxtesting.org).
>>
>>  Yes, this looks like a bug, thanks for finding it!
>>
>>  Please update your patch to use single exit point and not a mix of returns 
>> in the body of the function.
>>
>>>   ret = mutex_lock_interruptible(>bus_mutex);
>>>   if (ret != 0)
>>>  - goto post_unlock;
>>>  + return ret;
>>>
>>>   if (!sl->family_data) {
>>>  - ret = -ENODEV;
>>>  - goto pre_unlock;
>>>  + mutex_unlock(>bus_mutex);
>>>  + return -ENODEV;
>>>   }

Re: [PATCH] w1: keep balance of mutex locks and refcnts

[Evgeniy Polyakov]

Hi Alexey

07.10.2017, 20:59, "Alexey Khoroshilov" :
> Is it possible to switch to a nested variant:
> mutex_lock-atomic_inc-atomic_dec-mutex_unlock
> or
> atomic_inc-mutex_lock-mutex_unlock-atomic_dec
> ?

Yeah, you are right, it is a bit messy - we drop the lock while sleeping 
waiting for the bus master to complete operation,
and during this period family driver has to be referenced.

But we can easily grab the reference earlier and then try to lock the bus, so 
the second variant will work.

> --
> Alexey
>
> On 01.10.2017 08:55, Evgeniy Polyakov wrote:
>>  Hi Alex
>>
>>  29.09.2017, 23:23, "Alexey Khoroshilov" :
>>>  w1_therm_eeprom() and w1_DS18B20_precision() decrement THERM_REFCNT
>>>  on error paths, while they did not increment it yet.
>>>
>>>  read_therm() unlocks bus mutex on some error paths,
>>>  while it is not acquired.
>>>
>>>  The patch makes sure all the functions keep the balance in usage of
>>>  the mutex and the THERM_REFCNT.
>>>
>>>  Found by Linux Driver Verification project (linuxtesting.org).
>>
>>  Yes, this looks like a bug, thanks for finding it!
>>
>>  Please update your patch to use single exit point and not a mix of returns 
>> in the body of the function.
>>
>>>   ret = mutex_lock_interruptible(>bus_mutex);
>>>   if (ret != 0)
>>>  - goto post_unlock;
>>>  + return ret;
>>>
>>>   if (!sl->family_data) {
>>>  - ret = -ENODEV;
>>>  - goto pre_unlock;
>>>  + mutex_unlock(>bus_mutex);
>>>  + return -ENODEV;
>>>   }

Re: [PATCH v5 2/2] add w1_ds28e17 driver for the DS28E17 Onewire to I2C master bridge

[Evgeniy Polyakov]

Hi Jan, Greg

Have you resolved issues with the patch (touch bit export and driver itself)?
This driver lives outside the kernel quite for a while, do you need any help to 
resolve this?

21.09.2017, 00:52, "Jan Kandziora" <j...@gmx.de>:
> This subpatch adds a driver for the DS28E17 Onewire to I2C master bridge.
>
> Signed-off-by: Jan Kandziora <j...@gmx.de>
> Acked-by: Evgeniy Polyakov <z...@ioremap.net>
> ---
> Changes in v5 against v4 in this subpatch:
>   - adapted to linux-4.14-rc1
>
> Changes in v4 against v3 in this subpatch:
>   - adapted to linux-4.12.0
>
> Changes in v3 against v2 in this subpatch:
>   - fixed a bug in using the i2c_adapter_quirks structure
>
> Changes in v2 against v1 in this subpatch:
>   - added error handling in w1_f19_error()
>   - added struct i2c_adapter_quirks
>   - removed unnecessary checks of I2C address and read count
>   - cleaned up error codes
> + EOPNOTSUPP for read/write count == 0
> + ETIMEDOUT for busy timeout
> + ENXIO for no reply of I2C slave
> + EAGAIN for I2C invalid start condition
> + EIO for all w1 errors
>   - devm_kzalloc() instead of kzalloc()
>   - i2c speed to be set as 100, 400, 900 instead of 0, 1, 2
>   - added driver parameter documentation
>   - added sysfs documentation
>   - Kconfig fixed
>
>  Documentation/ABI/testing/sysfs-driver-w1_ds28e17 | 21 +
>  Documentation/w1/slaves/00-INDEX | 2 +
>  Documentation/w1/slaves/w1_ds28e17 | 68 ++
>  drivers/w1/slaves/Kconfig | 15 +
>  drivers/w1/slaves/Makefile | 1 +
>  drivers/w1/slaves/w1_ds28e17.c | 771 ++
>  6 files changed, 878 insertions(+)
>  create mode 100644 Documentation/ABI/testing/sysfs-driver-w1_ds28e17
>  create mode 100644 Documentation/w1/slaves/w1_ds28e17
>  create mode 100644 drivers/w1/slaves/w1_ds28e17.c
>
> diff --git a/Documentation/ABI/testing/sysfs-driver-w1_ds28e17 
> b/Documentation/ABI/testing/sysfs-driver-w1_ds28e17
> new file mode 100644
> index 000..d301e70
> --- /dev/null
> +++ b/Documentation/ABI/testing/sysfs-driver-w1_ds28e17
> @@ -0,0 +1,21 @@
> +What: /sys/bus/w1/devices/19-/speed
> +Date: Sep 2017
> +KernelVersion: 4.14
> +Contact: Jan Kandziora <j...@gmx.de>
> +Description: When written, this file sets the I2C speed on the connected
> + DS28E17 chip. When read, it reads the current setting from
> + the DS28E17 chip.
> + Valid values: 100, 400, 900 [kBaud].
> + Default 100, can be set by w1_ds28e17.speed= module parameter.
> +Users: w1_ds28e17 driver
> +
> +What: /sys/bus/w1/devices/19-/stretch
> +Date: Sep 2017
> +KernelVersion: 4.14
> +Contact: Jan Kandziora <j...@gmx.de>
> +Description: When written, this file sets the multiplier used to calculate
> + the busy timeout for I2C operations on the connected DS28E17
> + chip. When read, returns the current setting.
> + Valid values: 1 to 9.
> + Default 1, can be set by w1_ds28e17.stretch= module parameter.
> +Users: w1_ds28e17 driver
> diff --git a/Documentation/w1/slaves/00-INDEX 
> b/Documentation/w1/slaves/00-INDEX
> index 8d76718..68946f8 100644
> --- a/Documentation/w1/slaves/00-INDEX
> +++ b/Documentation/w1/slaves/00-INDEX
> @@ -10,3 +10,5 @@ w1_ds2438
>  - The Maxim/Dallas Semiconductor ds2438 smart battery monitor.
>  w1_ds28e04
>  - The Maxim/Dallas Semiconductor ds28e04 eeprom.
> +w1_ds28e17
> + - The Maxim/Dallas Semiconductor ds28e17 1-Wire-to-I2C Master Bridge.
> diff --git a/Documentation/w1/slaves/w1_ds28e17 
> b/Documentation/w1/slaves/w1_ds28e17
> new file mode 100644
> index 000..7fcfad5
> --- /dev/null
> +++ b/Documentation/w1/slaves/w1_ds28e17
> @@ -0,0 +1,68 @@
> +Kernel driver w1_ds28e17
> +
> +
> +Supported chips:
> + * Maxim DS28E17 1-Wire-to-I2C Master Bridge
> +
> +supported family codes:
> + W1_FAMILY_DS28E17 0x19
> +
> +Author: Jan Kandziora <j...@gmx.de>
> +
> +
> +Description
> +---
> +The DS28E17 is a Onewire slave device which acts as an I2C bus master.
> +
> +This driver creates a new I2C bus for any DS28E17 device detected. I2C buses
> +come and go as the DS28E17 devices come and go. I2C slave devices connected 
> to
> +a DS28E17 can be accessed by the kernel or userspace tools as if they were
> +connected to a "native" I2C bus master.
> +
> +
> +An udev rule like the following
> +---
> +SUBSYSTEM=="i2c-dev", KERNEL=="i2c-[0-9]*", ATTRS{name}=="w1-19-*", \
> + SYMLINK+="i2c-$attr{name}"
> +---
> +may b

Re: [PATCH v5 2/2] add w1_ds28e17 driver for the DS28E17 Onewire to I2C master bridge

[Evgeniy Polyakov]

Hi Jan, Greg

Have you resolved issues with the patch (touch bit export and driver itself)?
This driver lives outside the kernel quite for a while, do you need any help to 
resolve this?

21.09.2017, 00:52, "Jan Kandziora" :
> This subpatch adds a driver for the DS28E17 Onewire to I2C master bridge.
>
> Signed-off-by: Jan Kandziora 
> Acked-by: Evgeniy Polyakov 
> ---
> Changes in v5 against v4 in this subpatch:
>   - adapted to linux-4.14-rc1
>
> Changes in v4 against v3 in this subpatch:
>   - adapted to linux-4.12.0
>
> Changes in v3 against v2 in this subpatch:
>   - fixed a bug in using the i2c_adapter_quirks structure
>
> Changes in v2 against v1 in this subpatch:
>   - added error handling in w1_f19_error()
>   - added struct i2c_adapter_quirks
>   - removed unnecessary checks of I2C address and read count
>   - cleaned up error codes
> + EOPNOTSUPP for read/write count == 0
> + ETIMEDOUT for busy timeout
> + ENXIO for no reply of I2C slave
> + EAGAIN for I2C invalid start condition
> + EIO for all w1 errors
>   - devm_kzalloc() instead of kzalloc()
>   - i2c speed to be set as 100, 400, 900 instead of 0, 1, 2
>   - added driver parameter documentation
>   - added sysfs documentation
>   - Kconfig fixed
>
>  Documentation/ABI/testing/sysfs-driver-w1_ds28e17 | 21 +
>  Documentation/w1/slaves/00-INDEX | 2 +
>  Documentation/w1/slaves/w1_ds28e17 | 68 ++
>  drivers/w1/slaves/Kconfig | 15 +
>  drivers/w1/slaves/Makefile | 1 +
>  drivers/w1/slaves/w1_ds28e17.c | 771 ++
>  6 files changed, 878 insertions(+)
>  create mode 100644 Documentation/ABI/testing/sysfs-driver-w1_ds28e17
>  create mode 100644 Documentation/w1/slaves/w1_ds28e17
>  create mode 100644 drivers/w1/slaves/w1_ds28e17.c
>
> diff --git a/Documentation/ABI/testing/sysfs-driver-w1_ds28e17 
> b/Documentation/ABI/testing/sysfs-driver-w1_ds28e17
> new file mode 100644
> index 000..d301e70
> --- /dev/null
> +++ b/Documentation/ABI/testing/sysfs-driver-w1_ds28e17
> @@ -0,0 +1,21 @@
> +What: /sys/bus/w1/devices/19-/speed
> +Date: Sep 2017
> +KernelVersion: 4.14
> +Contact: Jan Kandziora 
> +Description: When written, this file sets the I2C speed on the connected
> + DS28E17 chip. When read, it reads the current setting from
> + the DS28E17 chip.
> + Valid values: 100, 400, 900 [kBaud].
> + Default 100, can be set by w1_ds28e17.speed= module parameter.
> +Users: w1_ds28e17 driver
> +
> +What: /sys/bus/w1/devices/19-/stretch
> +Date: Sep 2017
> +KernelVersion: 4.14
> +Contact: Jan Kandziora 
> +Description: When written, this file sets the multiplier used to calculate
> + the busy timeout for I2C operations on the connected DS28E17
> + chip. When read, returns the current setting.
> + Valid values: 1 to 9.
> + Default 1, can be set by w1_ds28e17.stretch= module parameter.
> +Users: w1_ds28e17 driver
> diff --git a/Documentation/w1/slaves/00-INDEX 
> b/Documentation/w1/slaves/00-INDEX
> index 8d76718..68946f8 100644
> --- a/Documentation/w1/slaves/00-INDEX
> +++ b/Documentation/w1/slaves/00-INDEX
> @@ -10,3 +10,5 @@ w1_ds2438
>  - The Maxim/Dallas Semiconductor ds2438 smart battery monitor.
>  w1_ds28e04
>  - The Maxim/Dallas Semiconductor ds28e04 eeprom.
> +w1_ds28e17
> + - The Maxim/Dallas Semiconductor ds28e17 1-Wire-to-I2C Master Bridge.
> diff --git a/Documentation/w1/slaves/w1_ds28e17 
> b/Documentation/w1/slaves/w1_ds28e17
> new file mode 100644
> index 000..7fcfad5
> --- /dev/null
> +++ b/Documentation/w1/slaves/w1_ds28e17
> @@ -0,0 +1,68 @@
> +Kernel driver w1_ds28e17
> +
> +
> +Supported chips:
> + * Maxim DS28E17 1-Wire-to-I2C Master Bridge
> +
> +supported family codes:
> + W1_FAMILY_DS28E17 0x19
> +
> +Author: Jan Kandziora 
> +
> +
> +Description
> +---
> +The DS28E17 is a Onewire slave device which acts as an I2C bus master.
> +
> +This driver creates a new I2C bus for any DS28E17 device detected. I2C buses
> +come and go as the DS28E17 devices come and go. I2C slave devices connected 
> to
> +a DS28E17 can be accessed by the kernel or userspace tools as if they were
> +connected to a "native" I2C bus master.
> +
> +
> +An udev rule like the following
> +---
> +SUBSYSTEM=="i2c-dev", KERNEL=="i2c-[0-9]*", ATTRS{name}=="w1-19-*", \
> + SYMLINK+="i2c-$attr{name}"
> +---
> +may be used to create stable /dev/i2c- entries based on the unique id of the
> +DS28E17 chip.
> +
> +
> +Driver p

Re: [PATCH 10/10] [RFC] w1_netlink.h: add support for nested structs

[Evgeniy Polyakov]

Hi

26.09.2017, 20:59, "Mauro Carvalho Chehab" :
> Describe nested struct/union fields
>
> NOTE: This is a pure test patch, meant to validate if the
> parsing logic for nested structs is working properly.
>
> I've no idea if the random text I added there is correct!

It looks correct, although I would switch master bus with bus master.

Feel free to add my acked-by.

> Signed-off-by: Mauro Carvalho Chehab 
> ---
>  drivers/w1/w1_netlink.h | 4 
>  1 file changed, 4 insertions(+)
>
> diff --git a/drivers/w1/w1_netlink.h b/drivers/w1/w1_netlink.h
> index a36661cd1f05..e781d1109cd7 100644
> --- a/drivers/w1/w1_netlink.h
> +++ b/drivers/w1/w1_netlink.h
> @@ -60,6 +60,10 @@ enum w1_netlink_message_types {
>   * @status: kernel feedback for success 0 or errno failure value
>   * @len: length of data following w1_netlink_msg
>   * @id: union holding master bus id (msg.id) and slave device id (id[8]).
> + * @id.id: Slave ID (8 bytes)
> + * @id.mst: master bus identification
> + * @id.mst.id: master bus ID
> + * @id.mst.res: master bus reserved
>   * @data: start address of any following data
>   *
>   * The base message structure for w1 messages over netlink.
> --
> 2.13.5

Re: [PATCH 10/10] [RFC] w1_netlink.h: add support for nested structs

[Evgeniy Polyakov]

Hi

26.09.2017, 20:59, "Mauro Carvalho Chehab" :
> Describe nested struct/union fields
>
> NOTE: This is a pure test patch, meant to validate if the
> parsing logic for nested structs is working properly.
>
> I've no idea if the random text I added there is correct!

It looks correct, although I would switch master bus with bus master.

Feel free to add my acked-by.

> Signed-off-by: Mauro Carvalho Chehab 
> ---
>  drivers/w1/w1_netlink.h | 4 
>  1 file changed, 4 insertions(+)
>
> diff --git a/drivers/w1/w1_netlink.h b/drivers/w1/w1_netlink.h
> index a36661cd1f05..e781d1109cd7 100644
> --- a/drivers/w1/w1_netlink.h
> +++ b/drivers/w1/w1_netlink.h
> @@ -60,6 +60,10 @@ enum w1_netlink_message_types {
>   * @status: kernel feedback for success 0 or errno failure value
>   * @len: length of data following w1_netlink_msg
>   * @id: union holding master bus id (msg.id) and slave device id (id[8]).
> + * @id.id: Slave ID (8 bytes)
> + * @id.mst: master bus identification
> + * @id.mst.id: master bus ID
> + * @id.mst.res: master bus reserved
>   * @data: start address of any following data
>   *
>   * The base message structure for w1 messages over netlink.
> --
> 2.13.5

Re: [PATCH] w1: keep balance of mutex locks and refcnts

[Evgeniy Polyakov]

Hi Alex

29.09.2017, 23:23, "Alexey Khoroshilov" :
> w1_therm_eeprom() and w1_DS18B20_precision() decrement THERM_REFCNT
> on error paths, while they did not increment it yet.
>
> read_therm() unlocks bus mutex on some error paths,
> while it is not acquired.
>
> The patch makes sure all the functions keep the balance in usage of
> the mutex and the THERM_REFCNT.
>
> Found by Linux Driver Verification project (linuxtesting.org).

Yes, this looks like a bug, thanks for finding it!

Please update your patch to use single exit point and not a mix of returns in 
the body of the function.

>  ret = mutex_lock_interruptible(>bus_mutex);
>  if (ret != 0)
> - goto post_unlock;
> + return ret;
>
>  if (!sl->family_data) {
> - ret = -ENODEV;
> - goto pre_unlock;
> + mutex_unlock(>bus_mutex);
> + return -ENODEV;
>  }

Re: [PATCH] w1: keep balance of mutex locks and refcnts

[Evgeniy Polyakov]

Hi Alex

29.09.2017, 23:23, "Alexey Khoroshilov" :
> w1_therm_eeprom() and w1_DS18B20_precision() decrement THERM_REFCNT
> on error paths, while they did not increment it yet.
>
> read_therm() unlocks bus mutex on some error paths,
> while it is not acquired.
>
> The patch makes sure all the functions keep the balance in usage of
> the mutex and the THERM_REFCNT.
>
> Found by Linux Driver Verification project (linuxtesting.org).

Yes, this looks like a bug, thanks for finding it!

Please update your patch to use single exit point and not a mix of returns in 
the body of the function.

>  ret = mutex_lock_interruptible(>bus_mutex);
>  if (ret != 0)
> - goto post_unlock;
> + return ret;
>
>  if (!sl->family_data) {
> - ret = -ENODEV;
> - goto pre_unlock;
> + mutex_unlock(>bus_mutex);
> + return -ENODEV;
>  }

Re: [PATCH v2] w1:fix byteorder of W1_READ_ROM id under big-endian cpu

[Evgeniy Polyakov]

Hi

28.08.2017, 12:25, "chen.l...@zte.com.cn" :
> Hi
>
> Q:
>
> But w1_reg_num has a different layout for be/le systems, isn't it enough?
>
> A:
>
> sure, it's right only under the assumption that 'rn' is a  correct layout id.
>
> Here's my example in be system which I encounter before.
>
> buf[0] return from w1_read_8(dev) in code section2 will always be 'family:8', 
> buf[0] store at the first byte of rn, then it will be transport to cb(dev, 
> rn) in code section1,
>
> but it will be parsed to 'crc:8' of the struct w1_reg_num in be system, then 
> there comes the wrong.

Sorry, I do not understand. Do you mean that there is a difference between 2 
ways to read ID content in w1_read_block() ?
If it is the case, is it possible, that there is a bug in particular master 
implementation?

There is a fair number of be devices in the tree already, and no one yet 
reported that there is an endian issue.

Re: [PATCH v2] w1:fix byteorder of W1_READ_ROM id under big-endian cpu

[Evgeniy Polyakov]

Hi

28.08.2017, 12:25, "chen.l...@zte.com.cn" :
> Hi
>
> Q:
>
> But w1_reg_num has a different layout for be/le systems, isn't it enough?
>
> A:
>
> sure, it's right only under the assumption that 'rn' is a  correct layout id.
>
> Here's my example in be system which I encounter before.
>
> buf[0] return from w1_read_8(dev) in code section2 will always be 'family:8', 
> buf[0] store at the first byte of rn, then it will be transport to cb(dev, 
> rn) in code section1,
>
> but it will be parsed to 'crc:8' of the struct w1_reg_num in be system, then 
> there comes the wrong.

Sorry, I do not understand. Do you mean that there is a difference between 2 
ways to read ID content in w1_read_block() ?
If it is the case, is it possible, that there is a bug in particular master 
implementation?

There is a fair number of be devices in the tree already, and no one yet 
reported that there is an endian issue.

Re: [PATCH] w1: ds2490: constify usb_device_id and fix space before '[' error

[Evgeniy Polyakov]

Hi Arvind

12.08.2017, 11:38, "Arvind Yadav" <arvind.yadav...@gmail.com>:
> usb_device_id are not supposed to change at runtime. All functions
> working with usb_device_id provided by  work with
> const usb_device_id. So mark the non-const structs as const.
>
> Fix checkpatch.pl error:
> ERROR: space prohibited before open square bracket '['.
>
> Signed-off-by: Arvind Yadav <arvind.yadav...@gmail.com>

Looks good to me, thank you
kernel-janitors@ please queue this up

Acked-by: Evgeniy Polyakov <z...@ioremap.net>

> ---
>  drivers/w1/masters/ds2490.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/w1/masters/ds2490.c b/drivers/w1/masters/ds2490.c
> index 46ccb2f..a02da9a 100644
> --- a/drivers/w1/masters/ds2490.c
> +++ b/drivers/w1/masters/ds2490.c
> @@ -1088,7 +1088,7 @@ static void ds_disconnect(struct usb_interface *intf)
>  kfree(dev);
>  }
>
> -static struct usb_device_id ds_id_table [] = {
> +static const struct usb_device_id ds_id_table[] = {
>  { USB_DEVICE(0x04fa, 0x2490) },
>  { },
>  };
> --
> 2.7.4

Re: [PATCH] w1: ds2490: constify usb_device_id and fix space before '[' error

[Evgeniy Polyakov]

Hi Arvind

12.08.2017, 11:38, "Arvind Yadav" :
> usb_device_id are not supposed to change at runtime. All functions
> working with usb_device_id provided by  work with
> const usb_device_id. So mark the non-const structs as const.
>
> Fix checkpatch.pl error:
> ERROR: space prohibited before open square bracket '['.
>
> Signed-off-by: Arvind Yadav 

Looks good to me, thank you
kernel-janitors@ please queue this up

Acked-by: Evgeniy Polyakov 

> ---
>  drivers/w1/masters/ds2490.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/w1/masters/ds2490.c b/drivers/w1/masters/ds2490.c
> index 46ccb2f..a02da9a 100644
> --- a/drivers/w1/masters/ds2490.c
> +++ b/drivers/w1/masters/ds2490.c
> @@ -1088,7 +1088,7 @@ static void ds_disconnect(struct usb_interface *intf)
>  kfree(dev);
>  }
>
> -static struct usb_device_id ds_id_table [] = {
> +static const struct usb_device_id ds_id_table[] = {
>  { USB_DEVICE(0x04fa, 0x2490) },
>  { },
>  };
> --
> 2.7.4

Re: [PATCH v4 2/2] add w1_ds28e17 driver for the DS28E17 Onewire to I2C master bridge

[Evgeniy Polyakov]

Hi Jan, Greg

19.07.2017, 23:14, "Jan Kandziora" <j...@gmx.de>:
> This subpatch adds a driver for the DS28E17 Onewire to I2C master bridge.
>
> Signed-off-by: Jan Kandziora <j...@gmx.de>

Greg, please queue this driver into your tree, it is sitting outside quite for 
a while and failed to be merged previously
since I missed it.
 
Acked-by: Evgeniy Polyakov <z...@ioremap.net>

> ---
> Changes in v4 against v3 in this subpatch:
>   - adapted to linux-4.12.0
>
> Changes in v3 against v2 in this subpatch:
>   - fixed a bug in using the i2c_adapter_quirks structure
>
> Changes in v2 against v1 in this subpatch:
>   - added error handling in w1_f19_error()
>   - added struct i2c_adapter_quirks
>   - removed unnecessary checks of I2C address and read count
>   - cleaned up error codes
> + EOPNOTSUPP for read/write count == 0
> + ETIMEDOUT for busy timeout
> + ENXIO for no reply of I2C slave
> + EAGAIN for I2C invalid start condition
> + EIO for all w1 errors
>   - devm_kzalloc() instead of kzalloc()
>   - i2c speed to be set as 100, 400, 900 instead of 0, 1, 2
>   - added driver parameter documentation
>   - added sysfs documentation
>   - Kconfig fixed
>
>  Documentation/ABI/testing/sysfs-driver-w1_ds28e17 | 21 +
>  Documentation/w1/slaves/00-INDEX | 2 +
>  Documentation/w1/slaves/w1_ds28e17 | 68 ++
>  drivers/w1/slaves/Kconfig | 15 +
>  drivers/w1/slaves/Makefile | 1 +
>  drivers/w1/slaves/w1_ds28e17.c | 772 ++
>  drivers/w1/w1_family.h | 1 +
>  7 files changed, 880 insertions(+)
>  create mode 100644 Documentation/ABI/testing/sysfs-driver-w1_ds28e17
>  create mode 100644 Documentation/w1/slaves/w1_ds28e17
>  create mode 100644 drivers/w1/slaves/w1_ds28e17.c
>
> diff --git a/Documentation/ABI/testing/sysfs-driver-w1_ds28e17 
> b/Documentation/ABI/testing/sysfs-driver-w1_ds28e17
> new file mode 100644
> index 000..b97b101
> --- /dev/null
> +++ b/Documentation/ABI/testing/sysfs-driver-w1_ds28e17
> @@ -0,0 +1,21 @@
> +What: /sys/bus/w1/devices/19-/speed
> +Date: Jul 2016
> +KernelVersion: 4.7
> +Contact: Jan Kandziora <j...@gmx.de>
> +Description: When written, this file sets the I2C speed on the connected
> + DS28E17 chip. When read, it reads the current setting from
> + the DS28E17 chip.
> + Valid values: 100, 400, 900 [kBaud].
> + Default 100, can be set by w1_ds28e17.speed= module parameter.
> +Users: w1_ds28e17 driver
> +
> +What: /sys/bus/w1/devices/19-/stretch
> +Date: Jul 2016
> +KernelVersion: 4.7
> +Contact: Jan Kandziora <j...@gmx.de>
> +Description: When written, this file sets the multiplier used to calculate
> + the busy timeout for I2C operations on the connected DS28E17
> + chip. When read, returns the current setting.
> + Valid values: 1 to 9.
> + Default 1, can be set by w1_ds28e17.stretch= module parameter.
> +Users: w1_ds28e17 driver
> diff --git a/Documentation/w1/slaves/00-INDEX 
> b/Documentation/w1/slaves/00-INDEX
> index 8d76718..68946f8 100644
> --- a/Documentation/w1/slaves/00-INDEX
> +++ b/Documentation/w1/slaves/00-INDEX
> @@ -10,3 +10,5 @@ w1_ds2438
>  - The Maxim/Dallas Semiconductor ds2438 smart battery monitor.
>  w1_ds28e04
>  - The Maxim/Dallas Semiconductor ds28e04 eeprom.
> +w1_ds28e17
> + - The Maxim/Dallas Semiconductor ds28e17 1-Wire-to-I2C Master Bridge.
> diff --git a/Documentation/w1/slaves/w1_ds28e17 
> b/Documentation/w1/slaves/w1_ds28e17
> new file mode 100644
> index 000..e82e16e
> --- /dev/null
> +++ b/Documentation/w1/slaves/w1_ds28e17
> @@ -0,0 +1,68 @@
> +Kernel driver w1_ds28e17
> +
> +
> +Supported chips:
> + * Maxim DS28E17 1-Wire-to-I2C Master Bridge
> +
> +supported family codes:
> + W1_FAMILY_DS28E17 0x19
> +
> +Author: Jan Kandziora <j...@gmx.de>
> +
> +
> +Description
> +---
> +The DS28E17 is a Onewire slave device which acts as an I2C bus master.
> +
> +This driver creates a new I2C bus for any DS28E17 device detected. I2C buses
> +come and go as the DS28E17 devices come and go. I2C slave devices connected 
> to
> +a DS28E17 can be accessed by the kernel or userspace tools as if they were
> +connected to a "native" I2C bus master.
> +
> +
> +An udev rule like the following
> +---
> +SUBSYSTEM=="i2c-dev", KERNEL=="i2c-[0-9]*", ATTRS{name}=="w1-19-*", \
> + SYMLINK+="i2c-$attr{name}"
> +---
> +may be used to create stable /dev/i2c- entries based on the unique id of the
> +

Re: [PATCH v4 2/2] add w1_ds28e17 driver for the DS28E17 Onewire to I2C master bridge

[Evgeniy Polyakov]

Hi Jan, Greg

19.07.2017, 23:14, "Jan Kandziora" :
> This subpatch adds a driver for the DS28E17 Onewire to I2C master bridge.
>
> Signed-off-by: Jan Kandziora 

Greg, please queue this driver into your tree, it is sitting outside quite for 
a while and failed to be merged previously
since I missed it.
 
Acked-by: Evgeniy Polyakov 

> ---
> Changes in v4 against v3 in this subpatch:
>   - adapted to linux-4.12.0
>
> Changes in v3 against v2 in this subpatch:
>   - fixed a bug in using the i2c_adapter_quirks structure
>
> Changes in v2 against v1 in this subpatch:
>   - added error handling in w1_f19_error()
>   - added struct i2c_adapter_quirks
>   - removed unnecessary checks of I2C address and read count
>   - cleaned up error codes
> + EOPNOTSUPP for read/write count == 0
> + ETIMEDOUT for busy timeout
> + ENXIO for no reply of I2C slave
> + EAGAIN for I2C invalid start condition
> + EIO for all w1 errors
>   - devm_kzalloc() instead of kzalloc()
>   - i2c speed to be set as 100, 400, 900 instead of 0, 1, 2
>   - added driver parameter documentation
>   - added sysfs documentation
>   - Kconfig fixed
>
>  Documentation/ABI/testing/sysfs-driver-w1_ds28e17 | 21 +
>  Documentation/w1/slaves/00-INDEX | 2 +
>  Documentation/w1/slaves/w1_ds28e17 | 68 ++
>  drivers/w1/slaves/Kconfig | 15 +
>  drivers/w1/slaves/Makefile | 1 +
>  drivers/w1/slaves/w1_ds28e17.c | 772 ++
>  drivers/w1/w1_family.h | 1 +
>  7 files changed, 880 insertions(+)
>  create mode 100644 Documentation/ABI/testing/sysfs-driver-w1_ds28e17
>  create mode 100644 Documentation/w1/slaves/w1_ds28e17
>  create mode 100644 drivers/w1/slaves/w1_ds28e17.c
>
> diff --git a/Documentation/ABI/testing/sysfs-driver-w1_ds28e17 
> b/Documentation/ABI/testing/sysfs-driver-w1_ds28e17
> new file mode 100644
> index 000..b97b101
> --- /dev/null
> +++ b/Documentation/ABI/testing/sysfs-driver-w1_ds28e17
> @@ -0,0 +1,21 @@
> +What: /sys/bus/w1/devices/19-/speed
> +Date: Jul 2016
> +KernelVersion: 4.7
> +Contact: Jan Kandziora 
> +Description: When written, this file sets the I2C speed on the connected
> + DS28E17 chip. When read, it reads the current setting from
> + the DS28E17 chip.
> + Valid values: 100, 400, 900 [kBaud].
> + Default 100, can be set by w1_ds28e17.speed= module parameter.
> +Users: w1_ds28e17 driver
> +
> +What: /sys/bus/w1/devices/19-/stretch
> +Date: Jul 2016
> +KernelVersion: 4.7
> +Contact: Jan Kandziora 
> +Description: When written, this file sets the multiplier used to calculate
> + the busy timeout for I2C operations on the connected DS28E17
> + chip. When read, returns the current setting.
> + Valid values: 1 to 9.
> + Default 1, can be set by w1_ds28e17.stretch= module parameter.
> +Users: w1_ds28e17 driver
> diff --git a/Documentation/w1/slaves/00-INDEX 
> b/Documentation/w1/slaves/00-INDEX
> index 8d76718..68946f8 100644
> --- a/Documentation/w1/slaves/00-INDEX
> +++ b/Documentation/w1/slaves/00-INDEX
> @@ -10,3 +10,5 @@ w1_ds2438
>  - The Maxim/Dallas Semiconductor ds2438 smart battery monitor.
>  w1_ds28e04
>  - The Maxim/Dallas Semiconductor ds28e04 eeprom.
> +w1_ds28e17
> + - The Maxim/Dallas Semiconductor ds28e17 1-Wire-to-I2C Master Bridge.
> diff --git a/Documentation/w1/slaves/w1_ds28e17 
> b/Documentation/w1/slaves/w1_ds28e17
> new file mode 100644
> index 000..e82e16e
> --- /dev/null
> +++ b/Documentation/w1/slaves/w1_ds28e17
> @@ -0,0 +1,68 @@
> +Kernel driver w1_ds28e17
> +
> +
> +Supported chips:
> + * Maxim DS28E17 1-Wire-to-I2C Master Bridge
> +
> +supported family codes:
> + W1_FAMILY_DS28E17 0x19
> +
> +Author: Jan Kandziora 
> +
> +
> +Description
> +---
> +The DS28E17 is a Onewire slave device which acts as an I2C bus master.
> +
> +This driver creates a new I2C bus for any DS28E17 device detected. I2C buses
> +come and go as the DS28E17 devices come and go. I2C slave devices connected 
> to
> +a DS28E17 can be accessed by the kernel or userspace tools as if they were
> +connected to a "native" I2C bus master.
> +
> +
> +An udev rule like the following
> +---
> +SUBSYSTEM=="i2c-dev", KERNEL=="i2c-[0-9]*", ATTRS{name}=="w1-19-*", \
> + SYMLINK+="i2c-$attr{name}"
> +---
> +may be used to create stable /dev/i2c- entries based on the unique id of the
> +DS28E17 chip.
> +
> +
> +Driver parameters are:
> +
> +speed:
> + This sets up the default I2C speed a DS28E

Re: [PATCH] connector: Delete an error message for a failed memory allocation in cn_queue_alloc_callback_entry()

[Evgeniy Polyakov]

Hi everyone

27.08.2017, 22:25, "SF Markus Elfring" <elfr...@users.sourceforge.net>:
> From: Markus Elfring <elfr...@users.sourceforge.net>
> Date: Sun, 27 Aug 2017 21:18:37 +0200
>
> Omit an extra message for a memory allocation failure in this function.
>
> This issue was detected by using the Coccinelle software.
>
> Signed-off-by: Markus Elfring <elfr...@users.sourceforge.net>

Looks good to me, thanks Markus.
There is virtually zero useful information in this print if we are in the 
situation, when kernel can not allocate
a few bytes to run connector queue.

Acked-by: Evgeniy Polyakov <z...@ioremap.net>

kernel-janitors@ please queue this patch up

> ---
>  drivers/connector/cn_queue.c | 4 +---
>  1 file changed, 1 insertion(+), 3 deletions(-)
>
> diff --git a/drivers/connector/cn_queue.c b/drivers/connector/cn_queue.c
> index 1f8bf054d11c..e4f31d679f02 100644
> --- a/drivers/connector/cn_queue.c
> +++ b/drivers/connector/cn_queue.c
> @@ -40,10 +40,8 @@ cn_queue_alloc_callback_entry(struct cn_queue_dev *dev, 
> const char *name,
>  struct cn_callback_entry *cbq;
>
>  cbq = kzalloc(sizeof(*cbq), GFP_KERNEL);
> - if (!cbq) {
> - pr_err("Failed to create new callback queue.\n");
> + if (!cbq)
>  return NULL;
> - }
>
>  atomic_set(>refcnt, 1);
>
> --
> 2.14.1

Re: [PATCH] connector: Delete an error message for a failed memory allocation in cn_queue_alloc_callback_entry()

[Evgeniy Polyakov]

Hi everyone

27.08.2017, 22:25, "SF Markus Elfring" :
> From: Markus Elfring 
> Date: Sun, 27 Aug 2017 21:18:37 +0200
>
> Omit an extra message for a memory allocation failure in this function.
>
> This issue was detected by using the Coccinelle software.
>
> Signed-off-by: Markus Elfring 

Looks good to me, thanks Markus.
There is virtually zero useful information in this print if we are in the 
situation, when kernel can not allocate
a few bytes to run connector queue.

Acked-by: Evgeniy Polyakov 

kernel-janitors@ please queue this patch up

> ---
>  drivers/connector/cn_queue.c | 4 +---
>  1 file changed, 1 insertion(+), 3 deletions(-)
>
> diff --git a/drivers/connector/cn_queue.c b/drivers/connector/cn_queue.c
> index 1f8bf054d11c..e4f31d679f02 100644
> --- a/drivers/connector/cn_queue.c
> +++ b/drivers/connector/cn_queue.c
> @@ -40,10 +40,8 @@ cn_queue_alloc_callback_entry(struct cn_queue_dev *dev, 
> const char *name,
>  struct cn_callback_entry *cbq;
>
>  cbq = kzalloc(sizeof(*cbq), GFP_KERNEL);
> - if (!cbq) {
> - pr_err("Failed to create new callback queue.\n");
> + if (!cbq)
>  return NULL;
> - }
>
>  atomic_set(>refcnt, 1);
>
> --
> 2.14.1

Re: [PATCH v2] w1:fix byteorder of W1_READ_ROM id under big-endian cpu

[Evgeniy Polyakov]

Hi Chen

19.07.2017, 10:58, "Chen Lin" :
> The byteorder of para rn(W1_READ_ROM id) pass to w1_slave_found must
> be the same with the byterorder defined in struct w1_reg_num.
>
> The rn read from 'rv = w1_read_block(dev, (u8 *), 8)' is a byte
> serial and not cpu endian relative, it need to change to cpu endian
> before passed to w1_slave_found.

But w1_reg_num has a different layout for be/le systems, isn't it enough?

> Signed-off-by: Chen Lin 
> Reviewed-by: Jiang Biao 
> ---
> Forgot the description in v1.
>
> ---
>  drivers/w1/w1.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/w1/w1.c b/drivers/w1/w1.c
> index 95ea7e6..c531545 100644
> --- a/drivers/w1/w1.c
> +++ b/drivers/w1/w1.c
> @@ -979,7 +979,7 @@ void w1_search(struct w1_master *dev, u8 search_type, 
> w1_slave_found_callback cb
>  mutex_unlock(>bus_mutex);
>
>  if (rv == 8 && rn)
> - cb(dev, rn);
> + cb(dev, le64_to_cpu(rn));
>
>  break;
>  }
> --
> 1.8.3.1

Re: [PATCH v2] w1:fix byteorder of W1_READ_ROM id under big-endian cpu

[Evgeniy Polyakov]

Hi Chen

19.07.2017, 10:58, "Chen Lin" :
> The byteorder of para rn(W1_READ_ROM id) pass to w1_slave_found must
> be the same with the byterorder defined in struct w1_reg_num.
>
> The rn read from 'rv = w1_read_block(dev, (u8 *), 8)' is a byte
> serial and not cpu endian relative, it need to change to cpu endian
> before passed to w1_slave_found.

But w1_reg_num has a different layout for be/le systems, isn't it enough?

> Signed-off-by: Chen Lin 
> Reviewed-by: Jiang Biao 
> ---
> Forgot the description in v1.
>
> ---
>  drivers/w1/w1.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/w1/w1.c b/drivers/w1/w1.c
> index 95ea7e6..c531545 100644
> --- a/drivers/w1/w1.c
> +++ b/drivers/w1/w1.c
> @@ -979,7 +979,7 @@ void w1_search(struct w1_master *dev, u8 search_type, 
> w1_slave_found_callback cb
>  mutex_unlock(>bus_mutex);
>
>  if (rv == 8 && rn)
> - cb(dev, rn);
> + cb(dev, le64_to_cpu(rn));
>
>  break;
>  }
> --
> 1.8.3.1

Re: [PATCH linux v5 0/3] Export 1-wire thermal sensors as hwmon device

[Evgeniy Polyakov]

Hi

18.07.2017, 02:09, "Jaghathiswari Rankappagounder Natarajan" :
> Hi Greg,
> Please pull in this patchset into the tree. Thanks!

Here is a patchset, is it visible in your mailbox?
If so, please pull it into your tree.

> Summary of what this patchset does:
>
> Our board has 4 DS18B20 1-wire temperature sensors. Each 1-wire bus and the
> sensor under it is already configured against the Linux 1-wire driver
> (called w1). They have a sysfs file(e.g.
> /sys/bus/w1/devices/w1_bus_master1/28-07704f4c/w1_slave) which, when read,
> runs code to read the temperature. We'd like the temperatures to show up in
> hwmon, so that the BMC IPMI sensor plumbing can forward those to host.
>
> This patchset is based on linux mainline version v4.10.
>
> Tested:
> Yes; On a board with 4 DS18B20 1-wire temperature sensors:
> root@zaius:/sys/class/hwmon# ls
> hwmon0 hwmon1 hwmon2 hwmon3 hwmon4 hwmon5
> root@zaius:/sys/class/hwmon# cd hwmon0
> root@zaius:/sys/class/hwmon/hwmon0# ls
> device name subsystem temp1_input uevent
> root@zaius:/sys/class/hwmon/hwmon0# cat temp1_input
> 24500
> root@zaius:/sys/class/hwmon/hwmon0# cd ..
> root@zaius:/sys/class/hwmon# cd hwmon1
> root@zaius:/sys/class/hwmon/hwmon1# cat temp1_input
> 26562
> root@zaius:/sys/class/hwmon/hwmon1# cd ..
> root@zaius:/sys/class/hwmon# cd hwmon2
> root@zaius:/sys/class/hwmon/hwmon2# cat temp1_input
> 27250
> root@zaius:/sys/class/hwmon/hwmon2# cd ..
> root@zaius:/sys/class/hwmon# cd hwmon3
> root@zaius:/sys/class/hwmon/hwmon3# cat temp1_input
> 22250
> root@zaius:/sys/class/hwmon/hwmon3#
>
> Jaghathiswari Rankappagounder Natarajan (3):
>   drivers: w1: add hwmon support structures
>   drivers: w1: refactor w1_slave_show to make the temp reading
> functionality separate
>   drivers: w1: add hwmon temp support for w1_therm
>
>  drivers/w1/slaves/w1_therm.c | 164 
> +++
>  drivers/w1/w1.c | 18 -
>  drivers/w1/w1.h | 2 +
>  drivers/w1/w1_family.h | 2 +
>  4 files changed, 156 insertions(+), 30 deletions(-)
>
> --
> 2.13.2.932.g7449e964c-goog

Re: [PATCH linux v5 0/3] Export 1-wire thermal sensors as hwmon device

[Evgeniy Polyakov]

Hi

18.07.2017, 02:09, "Jaghathiswari Rankappagounder Natarajan" :
> Hi Greg,
> Please pull in this patchset into the tree. Thanks!

Here is a patchset, is it visible in your mailbox?
If so, please pull it into your tree.

> Summary of what this patchset does:
>
> Our board has 4 DS18B20 1-wire temperature sensors. Each 1-wire bus and the
> sensor under it is already configured against the Linux 1-wire driver
> (called w1). They have a sysfs file(e.g.
> /sys/bus/w1/devices/w1_bus_master1/28-07704f4c/w1_slave) which, when read,
> runs code to read the temperature. We'd like the temperatures to show up in
> hwmon, so that the BMC IPMI sensor plumbing can forward those to host.
>
> This patchset is based on linux mainline version v4.10.
>
> Tested:
> Yes; On a board with 4 DS18B20 1-wire temperature sensors:
> root@zaius:/sys/class/hwmon# ls
> hwmon0 hwmon1 hwmon2 hwmon3 hwmon4 hwmon5
> root@zaius:/sys/class/hwmon# cd hwmon0
> root@zaius:/sys/class/hwmon/hwmon0# ls
> device name subsystem temp1_input uevent
> root@zaius:/sys/class/hwmon/hwmon0# cat temp1_input
> 24500
> root@zaius:/sys/class/hwmon/hwmon0# cd ..
> root@zaius:/sys/class/hwmon# cd hwmon1
> root@zaius:/sys/class/hwmon/hwmon1# cat temp1_input
> 26562
> root@zaius:/sys/class/hwmon/hwmon1# cd ..
> root@zaius:/sys/class/hwmon# cd hwmon2
> root@zaius:/sys/class/hwmon/hwmon2# cat temp1_input
> 27250
> root@zaius:/sys/class/hwmon/hwmon2# cd ..
> root@zaius:/sys/class/hwmon# cd hwmon3
> root@zaius:/sys/class/hwmon/hwmon3# cat temp1_input
> 22250
> root@zaius:/sys/class/hwmon/hwmon3#
>
> Jaghathiswari Rankappagounder Natarajan (3):
>   drivers: w1: add hwmon support structures
>   drivers: w1: refactor w1_slave_show to make the temp reading
> functionality separate
>   drivers: w1: add hwmon temp support for w1_therm
>
>  drivers/w1/slaves/w1_therm.c | 164 
> +++
>  drivers/w1/w1.c | 18 -
>  drivers/w1/w1.h | 2 +
>  drivers/w1/w1_family.h | 2 +
>  4 files changed, 156 insertions(+), 30 deletions(-)
>
> --
> 2.13.2.932.g7449e964c-goog

Re: [PATCH v4 1/2] wire: export w1_touch_bit

[Evgeniy Polyakov]

Hi

19.07.2017, 23:14, "Jan Kandziora" <j...@gmx.de>:
> The w1_ds28e17 driver from the next part of this patch needs to emit
> single-bit read timeslots to the DS28E17. The w1 subsystem already
> has this function but it is not exported outside drivers/w1/w1_io.c
>
> This subpatch exports the w1_touch_bit symbol with EXPORT_SYMBOL_GPL,
> same as the other exported symbols in drivers/w1/w1_io.c
>
> May be also useful later for writing drivers for other Onewire chips
> which do single-bit communication.
>
> Signed-off-by: Jan Kandziora <j...@gmx.de>
> ---
> Changes in v4 against v3 in this subpatch:
>   - adapted to linux-4.12.0

Greg, please pull this patch series into the tree, it has been adopted to the 
latest vanilla tree to date.
Acked-by: Evgeniy Polyakov <z...@ioremap.net>

Re: [PATCH v4 1/2] wire: export w1_touch_bit

[Evgeniy Polyakov]

Hi

19.07.2017, 23:14, "Jan Kandziora" :
> The w1_ds28e17 driver from the next part of this patch needs to emit
> single-bit read timeslots to the DS28E17. The w1 subsystem already
> has this function but it is not exported outside drivers/w1/w1_io.c
>
> This subpatch exports the w1_touch_bit symbol with EXPORT_SYMBOL_GPL,
> same as the other exported symbols in drivers/w1/w1_io.c
>
> May be also useful later for writing drivers for other Onewire chips
> which do single-bit communication.
>
> Signed-off-by: Jan Kandziora 
> ---
> Changes in v4 against v3 in this subpatch:
>   - adapted to linux-4.12.0

Greg, please pull this patch series into the tree, it has been adopted to the 
latest vanilla tree to date.
Acked-by: Evgeniy Polyakov 

Re: [PATCH v4 0/5] w1: ds1wm: register access patch

[Evgeniy Polyakov]

Hi everyone

25.07.2017, 14:27, "Johannes Poehlmann" :
> To make the ds1wm driver work on a powerpc architecture (big endian, 32bit)
> with a register offset multiplier of 4 I had to make some changes to

> Version 4 of the patchset
>    work on Greg Kroah-Hartmanns comments:
> o added changelog to every patch
> o separate one big patch into two
> o use bool
> o use blockcomments
> o dev_dbg instead of dev_info
> o rebased to v4.13-rc2

Greg, please pull this series named [PATCH v4 ...] w1: ds1wm ... into the tree, 
it looks ok

Re: [PATCH v4 0/5] w1: ds1wm: register access patch

[Evgeniy Polyakov]

Hi everyone

25.07.2017, 14:27, "Johannes Poehlmann" :
> To make the ds1wm driver work on a powerpc architecture (big endian, 32bit)
> with a register offset multiplier of 4 I had to make some changes to

> Version 4 of the patchset
>    work on Greg Kroah-Hartmanns comments:
> o added changelog to every patch
> o separate one big patch into two
> o use bool
> o use blockcomments
> o dev_dbg instead of dev_info
> o rebased to v4.13-rc2

Greg, please pull this series named [PATCH v4 ...] w1: ds1wm ... into the tree, 
it looks ok

Re: [PATCH v2 1/2] power: supply: move HDQ interface for bq27xxx from w1 to power/supply

[Evgeniy Polyakov]

Hi everyone

Sorry for long delay

25.07.2017, 16:48, "Sebastian Reichel" :
> Here is a signed immutable branch for Andrew's patch, which moves
> bq27000 w1 driver to the power-supply subsystem. I guess git will
> figure everything out without this, but better safe than sorry :)
>
> The following changes since commit 520eccdfe187591a51ea9ab4c1a024ae4d0f68d9:
>
>   Linux 4.13-rc2 (2017-07-23 16:15:17 -0700)
>
> are available in the git repository at:
>
>   git://git.kernel.org/pub/scm/linux/kernel/git/sre/linux-power-supply.git 
> tags/ib-w1-bq27000-4.14
>
> for you to fetch changes up to 55a9db679183bcf85a6e5c44a4f92f158bb6f03d:
>
>   power: supply: move HDQ interface for bq27xxx from w1 to power/supply 
> (2017-07-25 15:17:39 +0200)

Greg, should it be pulled into your tree or submitted as a separate pull 
request or as set of patches?
Either way looks good to me, please get it into your tree.

Re: [PATCH v2 1/2] power: supply: move HDQ interface for bq27xxx from w1 to power/supply

[Evgeniy Polyakov]

Hi everyone

Sorry for long delay

25.07.2017, 16:48, "Sebastian Reichel" :
> Here is a signed immutable branch for Andrew's patch, which moves
> bq27000 w1 driver to the power-supply subsystem. I guess git will
> figure everything out without this, but better safe than sorry :)
>
> The following changes since commit 520eccdfe187591a51ea9ab4c1a024ae4d0f68d9:
>
>   Linux 4.13-rc2 (2017-07-23 16:15:17 -0700)
>
> are available in the git repository at:
>
>   git://git.kernel.org/pub/scm/linux/kernel/git/sre/linux-power-supply.git 
> tags/ib-w1-bq27000-4.14
>
> for you to fetch changes up to 55a9db679183bcf85a6e5c44a4f92f158bb6f03d:
>
>   power: supply: move HDQ interface for bq27xxx from w1 to power/supply 
> (2017-07-25 15:17:39 +0200)

Greg, should it be pulled into your tree or submitted as a separate pull 
request or as set of patches?
Either way looks good to me, please get it into your tree.

Re: [PATCH linux v2 0/3] Export 1-wire thermal sensors as hwmon device

[Evgeniy Polyakov]

Hi

13.07.2017, 18:36, "Evgeniy Polyakov" <z...@ioremap.net>:
> I believe this is a resend of your previous patchet, isn't it?
> Greg, if you hadn't yet, please pull it into the tree.

Aaand it looks like we have a discussion now, so please postpone it for a while 
:)

Re: [PATCH linux v2 0/3] Export 1-wire thermal sensors as hwmon device

[Evgeniy Polyakov]

Hi

13.07.2017, 18:36, "Evgeniy Polyakov" :
> I believe this is a resend of your previous patchet, isn't it?
> Greg, if you hadn't yet, please pull it into the tree.

Aaand it looks like we have a discussion now, so please postpone it for a while 
:)

Re: [PATCH linux v2 0/3] Export 1-wire thermal sensors as hwmon device

[Evgeniy Polyakov]

Hi

13.07.2017, 00:41, "Jaghathiswari Rankappagounder Natarajan" :
> Our board has 4 DS18B20 1-wire temperature sensors. Each 1-wire bus and the
> sensor under it is already configured against the Linux 1-wire driver
> (called w1). They have a sysfs file(e.g.
> /sys/bus/w1/devices/w1_bus_master1/28-07704f4c/w1_slave) which, when read,
> runs code to read the temperature. We'd like the temperatures to show up in
> hwmon, so that the BMC IPMI sensor plumbing can forward those to host.
>
> The changes in version 2 are mentioned in the individual patches.
>
> This patchset is based on linux mainline version v4.10.

I believe this is a resend of your previous patchet, isn't it?
Greg, if you hadn't yet, please pull it into the tree.

Thank you.

Re: [PATCH linux v2 0/3] Export 1-wire thermal sensors as hwmon device

[Evgeniy Polyakov]

Hi

13.07.2017, 00:41, "Jaghathiswari Rankappagounder Natarajan" :
> Our board has 4 DS18B20 1-wire temperature sensors. Each 1-wire bus and the
> sensor under it is already configured against the Linux 1-wire driver
> (called w1). They have a sysfs file(e.g.
> /sys/bus/w1/devices/w1_bus_master1/28-07704f4c/w1_slave) which, when read,
> runs code to read the temperature. We'd like the temperatures to show up in
> hwmon, so that the BMC IPMI sensor plumbing can forward those to host.
>
> The changes in version 2 are mentioned in the individual patches.
>
> This patchset is based on linux mainline version v4.10.

I believe this is a resend of your previous patchet, isn't it?
Greg, if you hadn't yet, please pull it into the tree.

Thank you.

Re: [PATCH v4 1/2] wire: export w1_touch_bit

[Evgeniy Polyakov]

Hi Jan, Greg

11.07.2017, 02:00, "Jan Kandziora" <j...@gmx.de>:
> Changes in v4 against v3 in this subpatch:
>   - adapted to linux-4.12.0
>
> No changes in v3 against v2,v1 in this subpatch.
>
> The w1_ds28e17 driver from the next part of this patch needs to emit
> single-bit read timeslots to the DS28E17. The w1 subsystem already
> has this function but it is not exported outside drivers/w1/w1_io.c
>
> This subpatch exports the w1_touch_bit symbol with EXPORT_SYMBOL_GPL,
> same as the other exported symbols in drivers/w1/w1_io.c
>
> May be also useful later for writing drivers for other Onewire chips
> which do single-bit communication.
>
> Signed-off-by: Jan Kandziora <j...@gmx.de>

Greg, please pull this patchset into the tree, I missed it last time and it 
stuck in the queues for a too long.
Thank you.

Acked-by: Evgeniy Polyakov <z...@ioremap.net>

> ---
> From 6aedd1b890bd77bfe788f6af7a38724c16934ab0 Mon Sep 17 00:00:00 2001
> From: Jan Kandziora <j...@gmx.de>
> Date: Sat, 8 Jul 2017 21:14:27 +0200
> Subject: [PATCH 1/2] wire: export w1_touch_bit
>
> ---
>  drivers/w1/w1.h | 1 +
>  drivers/w1/w1_io.c | 3 ++-
>  2 files changed, 3 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/w1/w1.h b/drivers/w1/w1.h
> index 758a7a6..e7af855 100644
> --- a/drivers/w1/w1.h
> +++ b/drivers/w1/w1.h
> @@ -295,6 +295,7 @@ int w1_attach_slave_device(struct w1_master *dev,
> struct w1_reg_num *rn);
>  int w1_slave_detach(struct w1_slave *sl);
>   u8 w1_triplet(struct w1_master *dev, int bdir);
> +u8 w1_touch_bit(struct w1_master *dev, int bit);
>  void w1_write_8(struct w1_master *, u8);
>  u8 w1_read_8(struct w1_master *);
>  int w1_reset_bus(struct w1_master *);
> diff --git a/drivers/w1/w1_io.c b/drivers/w1/w1_io.c
> index 1134e6b..4bb77a1 100644
> --- a/drivers/w1/w1_io.c
> +++ b/drivers/w1/w1_io.c
> @@ -58,7 +58,7 @@ static u8 w1_read_bit(struct w1_master *dev);
>   * @dev: the master device
>   * @bit: 0 - write a 0, 1 - write a 0 read the level
>   */
> -static u8 w1_touch_bit(struct w1_master *dev, int bit)
> +u8 w1_touch_bit(struct w1_master *dev, int bit)
>  {
>  if (dev->bus_master->touch_bit)
>  return dev->bus_master->touch_bit(dev->bus_master->data, 
> bit);
> @@ -69,6 +69,7 @@ static u8 w1_touch_bit(struct w1_master *dev, int bit)
>  return 0;
>  }
>  }
> +EXPORT_SYMBOL_GPL(w1_touch_bit);
>   /**
>   * w1_write_bit() - Generates a write-0 or write-1 cycle.
> --
> 2.13.1

Re: [PATCH v4 1/2] wire: export w1_touch_bit

[Evgeniy Polyakov]

Hi Jan, Greg

11.07.2017, 02:00, "Jan Kandziora" :
> Changes in v4 against v3 in this subpatch:
>   - adapted to linux-4.12.0
>
> No changes in v3 against v2,v1 in this subpatch.
>
> The w1_ds28e17 driver from the next part of this patch needs to emit
> single-bit read timeslots to the DS28E17. The w1 subsystem already
> has this function but it is not exported outside drivers/w1/w1_io.c
>
> This subpatch exports the w1_touch_bit symbol with EXPORT_SYMBOL_GPL,
> same as the other exported symbols in drivers/w1/w1_io.c
>
> May be also useful later for writing drivers for other Onewire chips
> which do single-bit communication.
>
> Signed-off-by: Jan Kandziora 

Greg, please pull this patchset into the tree, I missed it last time and it 
stuck in the queues for a too long.
Thank you.

Acked-by: Evgeniy Polyakov 

> ---
> From 6aedd1b890bd77bfe788f6af7a38724c16934ab0 Mon Sep 17 00:00:00 2001
> From: Jan Kandziora 
> Date: Sat, 8 Jul 2017 21:14:27 +0200
> Subject: [PATCH 1/2] wire: export w1_touch_bit
>
> ---
>  drivers/w1/w1.h | 1 +
>  drivers/w1/w1_io.c | 3 ++-
>  2 files changed, 3 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/w1/w1.h b/drivers/w1/w1.h
> index 758a7a6..e7af855 100644
> --- a/drivers/w1/w1.h
> +++ b/drivers/w1/w1.h
> @@ -295,6 +295,7 @@ int w1_attach_slave_device(struct w1_master *dev,
> struct w1_reg_num *rn);
>  int w1_slave_detach(struct w1_slave *sl);
>   u8 w1_triplet(struct w1_master *dev, int bdir);
> +u8 w1_touch_bit(struct w1_master *dev, int bit);
>  void w1_write_8(struct w1_master *, u8);
>  u8 w1_read_8(struct w1_master *);
>  int w1_reset_bus(struct w1_master *);
> diff --git a/drivers/w1/w1_io.c b/drivers/w1/w1_io.c
> index 1134e6b..4bb77a1 100644
> --- a/drivers/w1/w1_io.c
> +++ b/drivers/w1/w1_io.c
> @@ -58,7 +58,7 @@ static u8 w1_read_bit(struct w1_master *dev);
>   * @dev: the master device
>   * @bit: 0 - write a 0, 1 - write a 0 read the level
>   */
> -static u8 w1_touch_bit(struct w1_master *dev, int bit)
> +u8 w1_touch_bit(struct w1_master *dev, int bit)
>  {
>  if (dev->bus_master->touch_bit)
>  return dev->bus_master->touch_bit(dev->bus_master->data, 
> bit);
> @@ -69,6 +69,7 @@ static u8 w1_touch_bit(struct w1_master *dev, int bit)
>  return 0;
>  }
>  }
> +EXPORT_SYMBOL_GPL(w1_touch_bit);
>   /**
>   * w1_write_bit() - Generates a write-0 or write-1 cycle.
> --
> 2.13.1

Re: [PATCH v2 0/4] w1: ds1wm: register access patch

[Evgeniy Polyakov]

Hi everyone

Greg, please pull this into your tree

23.06.2017, 13:47, "Johannes Poehlmann" :
> To make the ds1wm driver work on a powerpc architecture (big endian, 32bit)
> with a register offset multiplier of 4 I had to make some changes to
>
>    drivers/w1/masters/ds1wm.c
> and include/linux/mfd/ds1wm.h.
>
> Version 2 of the patchset
>
> o fixes kbuild reported build problems on x86_64
> o removes unobvious shift constants
> o moves shift value checking into the probe function
> o rename variable (fix 'CamelCase' style)
>
> Johannes Poehlmann:
>   w1: ds1wm: fix and simplify register access
>   w1: ds1wm: add level interrupt modes
>   w1: ds1wm: silence interrupts on HW before claiming the interrupt
>   w1: ds1wm: add messages to make incorporation in mfd-drivers easier
>
>  drivers/w1/masters/ds1wm.c | 109 
> ++---
>  include/linux/mfd/ds1wm.h | 9 
>  2 files changed, 111 insertions(+), 7 deletions(-)
>
> --
> 2.1.4

Re: [PATCH v2 0/4] w1: ds1wm: register access patch

[Evgeniy Polyakov]

Hi everyone

Greg, please pull this into your tree

23.06.2017, 13:47, "Johannes Poehlmann" :
> To make the ds1wm driver work on a powerpc architecture (big endian, 32bit)
> with a register offset multiplier of 4 I had to make some changes to
>
>    drivers/w1/masters/ds1wm.c
> and include/linux/mfd/ds1wm.h.
>
> Version 2 of the patchset
>
> o fixes kbuild reported build problems on x86_64
> o removes unobvious shift constants
> o moves shift value checking into the probe function
> o rename variable (fix 'CamelCase' style)
>
> Johannes Poehlmann:
>   w1: ds1wm: fix and simplify register access
>   w1: ds1wm: add level interrupt modes
>   w1: ds1wm: silence interrupts on HW before claiming the interrupt
>   w1: ds1wm: add messages to make incorporation in mfd-drivers easier
>
>  drivers/w1/masters/ds1wm.c | 109 
> ++---
>  include/linux/mfd/ds1wm.h | 9 
>  2 files changed, 111 insertions(+), 7 deletions(-)
>
> --
> 2.1.4

Re: [PATCH linux v1 0/3] Export 1-wire thermal sensors as hwmon device

[Evgeniy Polyakov]

Hi

15.06.2017, 00:59, "Jaghathiswari Rankappagounder Natarajan" <ja...@google.com>:
> Our board has 4 DS18B20 1-wire temperature sensors. Each 1-wire bus and the
> sensor under it is already configured against the Linux 1-wire driver
> (called w1). They have a sysfs file(e.g.
> /sys/bus/w1/devices/w1_bus_master1/28-07704f4c/w1_slave) which, when read,
> runs code to read the temperature. We'd like the temperatures to show up in
> hwmon, so that the BMC IPMI sensor plumbing can forward those to host.
>
> This patchset is based on linux mainline version v4.10.

Looks good to me, thank you.

Acked-by: Evgeniy Polyakov <z...@ioremap.net>

Re: [PATCH linux v1 0/3] Export 1-wire thermal sensors as hwmon device

[Evgeniy Polyakov]

Hi

15.06.2017, 00:59, "Jaghathiswari Rankappagounder Natarajan" :
> Our board has 4 DS18B20 1-wire temperature sensors. Each 1-wire bus and the
> sensor under it is already configured against the Linux 1-wire driver
> (called w1). They have a sysfs file(e.g.
> /sys/bus/w1/devices/w1_bus_master1/28-07704f4c/w1_slave) which, when read,
> runs code to read the temperature. We'd like the temperatures to show up in
> hwmon, so that the BMC IPMI sensor plumbing can forward those to host.
>
> This patchset is based on linux mainline version v4.10.

Looks good to me, thank you.

Acked-by: Evgeniy Polyakov 

Re: [PATCH] w1: w1-gpio: Fix double-free of platform_data

[Evgeniy Polyakov]

Hi Alexey

25.03.2017, 20:08, "Alexey Ignatov" :
> struct w1_gpio_platform_data was allocated using devres when using
> device tree. Then it was assigned to dev.platform_data, which leaded
> to double free on device removal by devres and by direct
> kfree(platform_data) in platform_device_release)

If this patch is still relevant, please add someone from device-tree into copy, 
I would think this bug
affect anyone and we see alot of bug reports since probe failure ends up with 
double free.

Also a nit:

> @@ -143,12 +141,12 @@ static int w1_gpio_probe(struct platform_device *pdev)
>  int err;
>
>  if (of_have_populated_dt()) {
> - err = w1_gpio_probe_dt(pdev);
> - if (err < 0)
> - return err;
> + pdata = w1_gpio_probe_dt(pdev);
> + if (IS_ERR(pdata) < 0)
> + return PTR_ERR(pdata);

IS_ERR() should not be used this way

Re: [PATCH] w1: w1-gpio: Fix double-free of platform_data

[Evgeniy Polyakov]

Hi Alexey

25.03.2017, 20:08, "Alexey Ignatov" :
> struct w1_gpio_platform_data was allocated using devres when using
> device tree. Then it was assigned to dev.platform_data, which leaded
> to double free on device removal by devres and by direct
> kfree(platform_data) in platform_device_release)

If this patch is still relevant, please add someone from device-tree into copy, 
I would think this bug
affect anyone and we see alot of bug reports since probe failure ends up with 
double free.

Also a nit:

> @@ -143,12 +141,12 @@ static int w1_gpio_probe(struct platform_device *pdev)
>  int err;
>
>  if (of_have_populated_dt()) {
> - err = w1_gpio_probe_dt(pdev);
> - if (err < 0)
> - return err;
> + pdata = w1_gpio_probe_dt(pdev);
> + if (IS_ERR(pdata) < 0)
> + return PTR_ERR(pdata);

IS_ERR() should not be used this way

Re: [PATCH v2 21/23] MAINTAINERS: Add file patterns for w1 device tree bindings

[Evgeniy Polyakov]

Hi

12.03.2017, 16:17, "Geert Uytterhoeven" <ge...@linux-m68k.org>:
> Submitters of device tree binding documentation may forget to CC
> the subsystem maintainer if this is missing.

I have no objection on this patch, but I do not really know who is responsible 
for this and which tree should it come into.
If it is ok, Greg, please pull it into your one, or I see devicetree@ mail in 
copy... Stumbled

> Signed-off-by: Geert Uytterhoeven <ge...@linux-m68k.org>
> Cc: Evgeniy Polyakov <z...@ioremap.net>
> Cc: Greg Kroah-Hartman <gre...@linuxfoundation.org>
> ---
> Please apply this patch directly if you want to be involved in device
> tree binding documentation for your subsystem.
>
> v2:
>   - No changes.
>
> Impact on next-20170310:
>
> +Evgeniy Polyakov <z...@ioremap.net> (maintainer:W1 DALLAS'S 1-WIRE BUS)
>  Rob Herring <robh...@kernel.org> (maintainer:OPEN FIRMWARE AND FLATTENED 
> DEVICE TREE BINDINGS)
>  Mark Rutland <mark.rutl...@arm.com> (maintainer:OPEN FIRMWARE AND FLATTENED 
> DEVICE TREE BINDINGS)
>  devicet...@vger.kernel.org (open list:OPEN FIRMWARE AND FLATTENED DEVICE 
> TREE BINDINGS)
>  linux-kernel@vger.kernel.org (open list)
> ---
>  MAINTAINERS | 1 +
>  1 file changed, 1 insertion(+)
>
> diff --git a/MAINTAINERS b/MAINTAINERS
> index 000342ab65971a8c..e80f00a3368587c7 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -13518,6 +13518,7 @@ F: drivers/mmc/host/vub300.c
>  W1 DALLAS'S 1-WIRE BUS
>  M: Evgeniy Polyakov <z...@ioremap.net>
>  S: Maintained
> +F: Documentation/devicetree/bindings/w1/
>  F: Documentation/w1/
>  F: drivers/w1/
>
> --
> 2.7.4

Re: [PATCH v2 21/23] MAINTAINERS: Add file patterns for w1 device tree bindings

[Evgeniy Polyakov]

Hi

12.03.2017, 16:17, "Geert Uytterhoeven" :
> Submitters of device tree binding documentation may forget to CC
> the subsystem maintainer if this is missing.

I have no objection on this patch, but I do not really know who is responsible 
for this and which tree should it come into.
If it is ok, Greg, please pull it into your one, or I see devicetree@ mail in 
copy... Stumbled

> Signed-off-by: Geert Uytterhoeven 
> Cc: Evgeniy Polyakov 
> Cc: Greg Kroah-Hartman 
> ---
> Please apply this patch directly if you want to be involved in device
> tree binding documentation for your subsystem.
>
> v2:
>   - No changes.
>
> Impact on next-20170310:
>
> +Evgeniy Polyakov  (maintainer:W1 DALLAS'S 1-WIRE BUS)
>  Rob Herring  (maintainer:OPEN FIRMWARE AND FLATTENED 
> DEVICE TREE BINDINGS)
>  Mark Rutland  (maintainer:OPEN FIRMWARE AND FLATTENED 
> DEVICE TREE BINDINGS)
>  devicet...@vger.kernel.org (open list:OPEN FIRMWARE AND FLATTENED DEVICE 
> TREE BINDINGS)
>  linux-kernel@vger.kernel.org (open list)
> ---
>  MAINTAINERS | 1 +
>  1 file changed, 1 insertion(+)
>
> diff --git a/MAINTAINERS b/MAINTAINERS
> index 000342ab65971a8c..e80f00a3368587c7 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -13518,6 +13518,7 @@ F: drivers/mmc/host/vub300.c
>  W1 DALLAS'S 1-WIRE BUS
>  M: Evgeniy Polyakov 
>  S: Maintained
> +F: Documentation/devicetree/bindings/w1/
>  F: Documentation/w1/
>  F: drivers/w1/
>
> --
> 2.7.4

Re: [PATCH v2 0/4] w1: add DS2438 support, documentation and small fixes

[Evgeniy Polyakov]

Hi everyone

23.02.2017, 09:38, "Mariusz Bialonczyk" :
> This is my second version of my w1 patchset.
> It mainly adds support for the DS2438. There is also a documentation
> for it and also a missing one for DS2413.

Looks good to me, thank you

Greg, please pull it into your tree

Re: [PATCH v2 0/4] w1: add DS2438 support, documentation and small fixes

[Evgeniy Polyakov]

Hi everyone

23.02.2017, 09:38, "Mariusz Bialonczyk" :
> This is my second version of my w1 patchset.
> It mainly adds support for the DS2438. There is also a documentation
> for it and also a missing one for DS2413.

Looks good to me, thank you

Greg, please pull it into your tree

Re: [PATCH] i2c: w1: Add devicetree match for DS2482

[Evgeniy Polyakov]

Hi Colin

22.01.2017, 20:55, "Colin Godsey" <crgod...@gmail.com>:
> Adds the OF devicetree match for the DS2482 I2C-1wire master and adds a 
> listing to the I2C trivial devicetree bindings doc. Based on a proposed patch 
> by Hector Palacios.

Looks good to me.
Acked-by: Evgeniy Polyakov <z...@ioremap.net>

Re: [PATCH] i2c: w1: Add devicetree match for DS2482

[Evgeniy Polyakov]

Hi Colin

22.01.2017, 20:55, "Colin Godsey" :
> Adds the OF devicetree match for the DS2482 I2C-1wire master and adds a 
> listing to the I2C trivial devicetree bindings doc. Based on a proposed patch 
> by Hector Palacios.

Looks good to me.
Acked-by: Evgeniy Polyakov 

Re: [PATCH] w1: omap_hdq: Free resources on error path

[Evgeniy Polyakov]

Hi Christophe

09.01.2017, 03:13, "Christophe JAILLET" <christophe.jail...@wanadoo.fr>:
> In case of error returned by '_omap_hdq_reset()', free resources as done
> elsewhere in this function.
>
> This patch slighly changes the semantic of the code. It now propagates the
> error code returned by '_omap_hdq_reset()' instead of returning -EINVAL
> unconditionally.
>
> Signed-off-by: Christophe JAILLET <christophe.jail...@wanadoo.fr>

Looks good to me.
If kernel-janitors@ will not push it upstream feel free to send to to GregKH 
<g...@kroah.com>
and add me to copy.

Acked-by: Evgeniy Polyakov <z...@ioremap.net>

> ---
>  drivers/w1/masters/omap_hdq.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/w1/masters/omap_hdq.c b/drivers/w1/masters/omap_hdq.c
> index bb09de633939..fb190c259607 100644
> --- a/drivers/w1/masters/omap_hdq.c
> +++ b/drivers/w1/masters/omap_hdq.c
> @@ -715,7 +715,7 @@ static int omap_hdq_probe(struct platform_device *pdev)
>  ret = _omap_hdq_reset(hdq_data);
>  if (ret) {
>  dev_dbg(>dev, "reset failed\n");
> - return -EINVAL;
> + goto err_irq;
>  }
>
>  rev = hdq_reg_in(hdq_data, OMAP_HDQ_REVISION);
> --
> 2.9.3

Re: [PATCH] w1: omap_hdq: Free resources on error path

[Evgeniy Polyakov]

Hi Christophe

09.01.2017, 03:13, "Christophe JAILLET" :
> In case of error returned by '_omap_hdq_reset()', free resources as done
> elsewhere in this function.
>
> This patch slighly changes the semantic of the code. It now propagates the
> error code returned by '_omap_hdq_reset()' instead of returning -EINVAL
> unconditionally.
>
> Signed-off-by: Christophe JAILLET 

Looks good to me.
If kernel-janitors@ will not push it upstream feel free to send to to GregKH 

and add me to copy.

Acked-by: Evgeniy Polyakov 

> ---
>  drivers/w1/masters/omap_hdq.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/w1/masters/omap_hdq.c b/drivers/w1/masters/omap_hdq.c
> index bb09de633939..fb190c259607 100644
> --- a/drivers/w1/masters/omap_hdq.c
> +++ b/drivers/w1/masters/omap_hdq.c
> @@ -715,7 +715,7 @@ static int omap_hdq_probe(struct platform_device *pdev)
>  ret = _omap_hdq_reset(hdq_data);
>  if (ret) {
>  dev_dbg(>dev, "reset failed\n");
> - return -EINVAL;
> + goto err_irq;
>  }
>
>  rev = hdq_reg_in(hdq_data, OMAP_HDQ_REVISION);
> --
> 2.9.3

Re: [PATCH] [RFC] proc connector: add namespace events

[Evgeniy Polyakov]

Hi everyone

08.09.2016, 18:39, "Alban Crequy" :
> The act of a process creating or joining a namespace via clone(),
> unshare() or setns() is a useful signal for monitoring applications.

> + if (old_ns->mnt_ns != new_ns->mnt_ns)
> + proc_ns_connector(tsk, CLONE_NEWNS, PROC_NM_REASON_CLONE, old_mntns_inum, 
> new_mntns_inum);
> +
> + if (old_ns->uts_ns != new_ns->uts_ns)
> + proc_ns_connector(tsk, CLONE_NEWUTS, PROC_NM_REASON_CLONE, 
> old_ns->uts_ns->ns.inum, new_ns->uts_ns->ns.inum);
> +
> + if (old_ns->ipc_ns != new_ns->ipc_ns)
> + proc_ns_connector(tsk, CLONE_NEWIPC, PROC_NM_REASON_CLONE, 
> old_ns->ipc_ns->ns.inum, new_ns->ipc_ns->ns.inum);
> +
> + if (old_ns->net_ns != new_ns->net_ns)
> + proc_ns_connector(tsk, CLONE_NEWNET, PROC_NM_REASON_CLONE, 
> old_ns->net_ns->ns.inum, new_ns->net_ns->ns.inum);
> +
> + if (old_ns->cgroup_ns != new_ns->cgroup_ns)
> + proc_ns_connector(tsk, CLONE_NEWCGROUP, PROC_NM_REASON_CLONE, 
> old_ns->cgroup_ns->ns.inum, new_ns->cgroup_ns->ns.inum);
> +
> + if (old_ns->pid_ns_for_children != new_ns->pid_ns_for_children)
> + proc_ns_connector(tsk, CLONE_NEWPID, PROC_NM_REASON_CLONE, 
> old_ns->pid_ns_for_children->ns.inum, new_ns->pid_ns_for_children->ns.inum);
> + }
> +

Patch looks good to me from technical/connector point of view, but these even 
multiplication is a bit weird imho.

I'm not against it, but did you consider sending just 2 serialized ns 
structures via single message, and client
would check all ns bits himself?

Re: [PATCH] [RFC] proc connector: add namespace events

[Evgeniy Polyakov]

Hi everyone

08.09.2016, 18:39, "Alban Crequy" :
> The act of a process creating or joining a namespace via clone(),
> unshare() or setns() is a useful signal for monitoring applications.

> + if (old_ns->mnt_ns != new_ns->mnt_ns)
> + proc_ns_connector(tsk, CLONE_NEWNS, PROC_NM_REASON_CLONE, old_mntns_inum, 
> new_mntns_inum);
> +
> + if (old_ns->uts_ns != new_ns->uts_ns)
> + proc_ns_connector(tsk, CLONE_NEWUTS, PROC_NM_REASON_CLONE, 
> old_ns->uts_ns->ns.inum, new_ns->uts_ns->ns.inum);
> +
> + if (old_ns->ipc_ns != new_ns->ipc_ns)
> + proc_ns_connector(tsk, CLONE_NEWIPC, PROC_NM_REASON_CLONE, 
> old_ns->ipc_ns->ns.inum, new_ns->ipc_ns->ns.inum);
> +
> + if (old_ns->net_ns != new_ns->net_ns)
> + proc_ns_connector(tsk, CLONE_NEWNET, PROC_NM_REASON_CLONE, 
> old_ns->net_ns->ns.inum, new_ns->net_ns->ns.inum);
> +
> + if (old_ns->cgroup_ns != new_ns->cgroup_ns)
> + proc_ns_connector(tsk, CLONE_NEWCGROUP, PROC_NM_REASON_CLONE, 
> old_ns->cgroup_ns->ns.inum, new_ns->cgroup_ns->ns.inum);
> +
> + if (old_ns->pid_ns_for_children != new_ns->pid_ns_for_children)
> + proc_ns_connector(tsk, CLONE_NEWPID, PROC_NM_REASON_CLONE, 
> old_ns->pid_ns_for_children->ns.inum, new_ns->pid_ns_for_children->ns.inum);
> + }
> +

Patch looks good to me from technical/connector point of view, but these even 
multiplication is a bit weird imho.

I'm not against it, but did you consider sending just 2 serialized ns 
structures via single message, and client
would check all ns bits himself?

Re: [PATCH v2] w1: fix typo in parameter description

[Evgeniy Polyakov]

Hi

11.08.2016, 03:03, "Wei Yongjun" <weiyj...@gmail.com>:
> Fix typo in parameter description.
>
> Signed-off-by: Wei Yongjun <weiyj...@gmail.com>

Looks good to me, thank you
Greg, please pull it into your tree

Acked-by: Evgeniy Polyakov <z...@ioremap.net>

> ---
> v1 -> v2: make it as one line, comments from Evgeniy Polyakov
> ---
>  drivers/w1/w1.c | 3 +--
>  1 file changed, 1 insertion(+), 2 deletions(-)
>
> diff --git a/drivers/w1/w1.c b/drivers/w1/w1.c
> index bb34362..4bd898b 100644
> --- a/drivers/w1/w1.c
> +++ b/drivers/w1/w1.c
> @@ -53,8 +53,7 @@ int w1_max_slave_ttl = 10;
>  module_param_named(timeout, w1_timeout, int, 0);
>  MODULE_PARM_DESC(timeout, "time in seconds between automatic slave 
> searches");
>  module_param_named(timeout_us, w1_timeout_us, int, 0);
> -MODULE_PARM_DESC(timeout, "time in microseconds between automatic slave"
> - " searches");
> +MODULE_PARM_DESC(timeout_us, "time in microseconds between automatic slave 
> searches");
>  /* A search stops when w1_max_slave_count devices have been found in that
>   * search. The next search will start over and detect the same set of devices
>   * on a static 1-wire bus. Memory is not allocated based on this number, just

Re: [PATCH v2] w1: fix typo in parameter description

[Evgeniy Polyakov]

Hi

11.08.2016, 03:03, "Wei Yongjun" :
> Fix typo in parameter description.
>
> Signed-off-by: Wei Yongjun 

Looks good to me, thank you
Greg, please pull it into your tree

Acked-by: Evgeniy Polyakov 

> ---
> v1 -> v2: make it as one line, comments from Evgeniy Polyakov
> ---
>  drivers/w1/w1.c | 3 +--
>  1 file changed, 1 insertion(+), 2 deletions(-)
>
> diff --git a/drivers/w1/w1.c b/drivers/w1/w1.c
> index bb34362..4bd898b 100644
> --- a/drivers/w1/w1.c
> +++ b/drivers/w1/w1.c
> @@ -53,8 +53,7 @@ int w1_max_slave_ttl = 10;
>  module_param_named(timeout, w1_timeout, int, 0);
>  MODULE_PARM_DESC(timeout, "time in seconds between automatic slave 
> searches");
>  module_param_named(timeout_us, w1_timeout_us, int, 0);
> -MODULE_PARM_DESC(timeout, "time in microseconds between automatic slave"
> - " searches");
> +MODULE_PARM_DESC(timeout_us, "time in microseconds between automatic slave 
> searches");
>  /* A search stops when w1_max_slave_count devices have been found in that
>   * search. The next search will start over and detect the same set of devices
>   * on a static 1-wire bus. Memory is not allocated based on this number, just

Re: [PATCH] w1: fix timeout_us parameter description

[Evgeniy Polyakov]

Hi

10.08.2016, 06:22, "Wei Yongjun" :
>>  08.08.2016, 16:52, "Wei Yongjun" :
>>>  Fix 'timeout_us' parameter description.

>>>   MODULE_PARM_DESC(timeout, "time in seconds between automatic slave 
>>> searches");
>>>   module_param_named(timeout_us, w1_timeout_us, int, 0);
>>>  -MODULE_PARM_DESC(timeout, "time in microseconds between automatic slave"
>>>  - " searches");
>>>  +MODULE_PARM_DESC(timeout_us,
>>>  + "time in microseconds between automatic slave searches");
>>  I believe there will be no harm to put it to on one line, even if it 
>> crosses some obscure very-long-line rule
>
> Maybe the bad patch description confused you, this patch the typo in the
> first argument of MODULE_PARM_DESC(), use timeout_us instead of timeout.

Yup, you are right, please make it as one line since you are at it

Re: [PATCH] w1: fix timeout_us parameter description

[Evgeniy Polyakov]

Hi

10.08.2016, 06:22, "Wei Yongjun" :
>>  08.08.2016, 16:52, "Wei Yongjun" :
>>>  Fix 'timeout_us' parameter description.

>>>   MODULE_PARM_DESC(timeout, "time in seconds between automatic slave 
>>> searches");
>>>   module_param_named(timeout_us, w1_timeout_us, int, 0);
>>>  -MODULE_PARM_DESC(timeout, "time in microseconds between automatic slave"
>>>  - " searches");
>>>  +MODULE_PARM_DESC(timeout_us,
>>>  + "time in microseconds between automatic slave searches");
>>  I believe there will be no harm to put it to on one line, even if it 
>> crosses some obscure very-long-line rule
>
> Maybe the bad patch description confused you, this patch the typo in the
> first argument of MODULE_PARM_DESC(), use timeout_us instead of timeout.

Yup, you are right, please make it as one line since you are at it

Re: [PATCH] w1: fix timeout_us parameter description

[Evgeniy Polyakov]

Hi

08.08.2016, 16:52, "Wei Yongjun" :
> Fix 'timeout_us' parameter description.
>
> Signed-off-by: Wei Yongjun 
> ---
>  drivers/w1/w1.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/w1/w1.c b/drivers/w1/w1.c
> index bb34362..e213c67 100644
> --- a/drivers/w1/w1.c
> +++ b/drivers/w1/w1.c
> @@ -53,8 +53,8 @@ int w1_max_slave_ttl = 10;
>  module_param_named(timeout, w1_timeout, int, 0);
>  MODULE_PARM_DESC(timeout, "time in seconds between automatic slave 
> searches");
>  module_param_named(timeout_us, w1_timeout_us, int, 0);
> -MODULE_PARM_DESC(timeout, "time in microseconds between automatic slave"
> - " searches");
> +MODULE_PARM_DESC(timeout_us,
> + "time in microseconds between automatic slave searches");

I believe there will be no harm to put it to on one line, even if it crosses 
some obscure very-long-line rule

Re: [PATCH] w1: fix timeout_us parameter description

[Evgeniy Polyakov]

Hi

08.08.2016, 16:52, "Wei Yongjun" :
> Fix 'timeout_us' parameter description.
>
> Signed-off-by: Wei Yongjun 
> ---
>  drivers/w1/w1.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/w1/w1.c b/drivers/w1/w1.c
> index bb34362..e213c67 100644
> --- a/drivers/w1/w1.c
> +++ b/drivers/w1/w1.c
> @@ -53,8 +53,8 @@ int w1_max_slave_ttl = 10;
>  module_param_named(timeout, w1_timeout, int, 0);
>  MODULE_PARM_DESC(timeout, "time in seconds between automatic slave 
> searches");
>  module_param_named(timeout_us, w1_timeout_us, int, 0);
> -MODULE_PARM_DESC(timeout, "time in microseconds between automatic slave"
> - " searches");
> +MODULE_PARM_DESC(timeout_us,
> + "time in microseconds between automatic slave searches");

I believe there will be no harm to put it to on one line, even if it crosses 
some obscure very-long-line rule

Re: [PATCH] w1: Remove unneeded header file

[Evgeniy Polyakov]

Hi

28.06.2016, 16:50, "Julia Lawall" :
>>  24.06.2016, 09:18, "Amitoj Kaur Chawla" :
>>  > The Coccinelle semantic patch used to make this change is as follows:
>>  > @ includesmodule @
>>  > @@
>>  >
>>  > #include 
>>  >
>>  > @ depends on includesmodule @
>>  > @@
>>  >
>>  > - #include 
>>  >
>>  > Signed-off-by: Amitoj Kaur Chawla 
>>
>>  Looks good to me, but these @@ are pretty cryptic.
>
> There are two rules. The first checks for an include of module.h. The
> second depends on the success of the first one. It removes and include o
> moduleparam.h.

Ok, I see, it makes much more sense now :)

Thank you for clarification

Re: [PATCH] w1: Remove unneeded header file

[Evgeniy Polyakov]

Hi

28.06.2016, 16:50, "Julia Lawall" :
>>  24.06.2016, 09:18, "Amitoj Kaur Chawla" :
>>  > The Coccinelle semantic patch used to make this change is as follows:
>>  > @ includesmodule @
>>  > @@
>>  >
>>  > #include 
>>  >
>>  > @ depends on includesmodule @
>>  > @@
>>  >
>>  > - #include 
>>  >
>>  > Signed-off-by: Amitoj Kaur Chawla 
>>
>>  Looks good to me, but these @@ are pretty cryptic.
>
> There are two rules. The first checks for an include of module.h. The
> second depends on the success of the first one. It removes and include o
> moduleparam.h.

Ok, I see, it makes much more sense now :)

Thank you for clarification

Re: [PATCH] w1: Remove unneeded header file

[Evgeniy Polyakov]

Hi

24.06.2016, 09:18, "Amitoj Kaur Chawla" :
> Drop redundant include of moduleparam.h
>
> The Coccinelle semantic patch used to make this change is as follows:
> @ includesmodule @
> @@
>
> #include 
>
> @ depends on includesmodule @
> @@
>
> - #include 
>
> Signed-off-by: Amitoj Kaur Chawla 

Looks good to me, but these @@ are pretty cryptic.

Re: [PATCH] w1: Remove unneeded header file

[Evgeniy Polyakov]

Hi

24.06.2016, 09:18, "Amitoj Kaur Chawla" :
> Drop redundant include of moduleparam.h
>
> The Coccinelle semantic patch used to make this change is as follows:
> @ includesmodule @
> @@
>
> #include 
>
> @ depends on includesmodule @
> @@
>
> - #include 
>
> Signed-off-by: Amitoj Kaur Chawla 

Looks good to me, but these @@ are pretty cryptic.

Re: [PATCH] connector: fix out-of-order cn_proc netlink message delivery

[Evgeniy Polyakov]

Hi Aaron

24.06.2016, 16:07, "Aaron Campbell" :
> The proc connector messages include a sequence number, allowing userspace
> programs to detect lost messages. However, performing this detection is
> currently more difficult than necessary, since netlink messages can be
> delivered to the application out-of-order. To fix this, leave pre-emption
> disabled during cn_netlink_send(), and use GFP_NOWAIT.
>
> The following was written as a test case. Building the kernel w/ make -j32
> proved a reliable way to generate out-of-order cn_proc messages.

This is not actually about out-of-order sending which is impossible iirc,
but the way fork pushes messages into socket queue in parallel. What you've done
is syncing one more layer higher.

I'm not against this patch if you think it does fix some issues, but wording is 
not correct imo.

Re: [PATCH] connector: fix out-of-order cn_proc netlink message delivery

[Evgeniy Polyakov]

Hi Aaron

24.06.2016, 16:07, "Aaron Campbell" :
> The proc connector messages include a sequence number, allowing userspace
> programs to detect lost messages. However, performing this detection is
> currently more difficult than necessary, since netlink messages can be
> delivered to the application out-of-order. To fix this, leave pre-emption
> disabled during cn_netlink_send(), and use GFP_NOWAIT.
>
> The following was written as a test case. Building the kernel w/ make -j32
> proved a reliable way to generate out-of-order cn_proc messages.

This is not actually about out-of-order sending which is impossible iirc,
but the way fork pushes messages into socket queue in parallel. What you've done
is syncing one more layer higher.

I'm not against this patch if you think it does fix some issues, but wording is 
not correct imo.

Re: [PATCH] w1: add ability to set (SRAM) and store (EEPROM) configuration for temp sensors like DS18B20

[Evgeniy Polyakov]

Hi

17.04.2016, 02:08, "x29a" <0.x29...@googlemail.com>:
> based on the PR (https://github.com/raspberrypi/linux/pull/1412) for the 
> raspberry pi kernel, i would like to propose my changes upstream as well.
>
> The changes entitle basic write operations for the 1Wire driver which are 
> needed to change e.g. the precision of temperature sensors like the very 
> popular DS18B20. Writing to SRAM and commiting the values to EEPROM is 
> possible.
>
> Since many sensors come "preconfigured" with a lower precision, people are 
> stuck that precision when running on a kernel based device (unlike the Dallas 
> 1Wire library for e.g. Arduino, which supports writing the 
> configuration/scratchpad).

This looks good to me, Greg, please pull it into your tree

> Signed-off-by: x29a <0.x29...@gmail.com>

Acked-by: Evgeniy Polyakov <z...@ioremap.net>

> ---
>  Documentation/w1/slaves/w1_therm | 10 +-
>  drivers/w1/slaves/w1_therm.c | 218 +--
>  drivers/w1/w1.h | 2 +
>  3 files changed, 222 insertions(+), 8 deletions(-)
>
> diff --git a/Documentation/w1/slaves/w1_therm 
> b/Documentation/w1/slaves/w1_therm
> index 13411fe..d1f93af 100644
> --- a/Documentation/w1/slaves/w1_therm
> +++ b/Documentation/w1/slaves/w1_therm
> @@ -33,7 +33,15 @@ temperature conversion at a time. If none of the devices 
> are parasite
>  powered it would be possible to convert all the devices at the same
>  time and then go back to read individual sensors. That isn't
>  currently supported. The driver also doesn't support reduced
> -precision (which would also reduce the conversion time).
> +precision (which would also reduce the conversion time) when reading values.
> +
> +Writing a value between 9 and 12 to the sysfs w1_slave file will change the
> +precision of the sensor for the next readings. This value is in (volatile)
> +SRAM, so it is reset when the sensor gets power-cycled.
> +
> +To store the current precision configuration into EEPROM, the value 0
> +has to be written to the sysfs w1_slave file. Since the EEPROM has a limited
> +amount of writes (>50k), this command should be used wisely.
>
>  The module parameter strong_pullup can be set to 0 to disable the
>  strong pullup, 1 to enable autodetection or 2 to force strong pullup.
> diff --git a/drivers/w1/slaves/w1_therm.c b/drivers/w1/slaves/w1_therm.c
> index 2f029e8..581a300 100644
> --- a/drivers/w1/slaves/w1_therm.c
> +++ b/drivers/w1/slaves/w1_therm.c
> @@ -92,10 +92,13 @@ static void w1_therm_remove_slave(struct w1_slave *sl)
>  static ssize_t w1_slave_show(struct device *device,
>  struct device_attribute *attr, char *buf);
>
> +static ssize_t w1_slave_store(struct device *device,
> + struct device_attribute *attr, const char *buf, size_t size);
> +
>  static ssize_t w1_seq_show(struct device *device,
>  struct device_attribute *attr, char *buf);
>
> -static DEVICE_ATTR_RO(w1_slave);
> +static DEVICE_ATTR_RW(w1_slave);
>  static DEVICE_ATTR_RO(w1_seq);
>
>  static struct attribute *w1_therm_attrs[] = {
> @@ -154,8 +157,17 @@ struct w1_therm_family_converter
>  u16 reserved;
>  struct w1_family *f;
>  int (*convert)(u8 rom[9]);
> + int (*precision)(struct device *device, int val);
> + int (*eeprom)(struct device *device);
>  };
>
> +/* write configuration to eeprom */
> +static inline int w1_therm_eeprom(struct device *device);
> +
> +/* Set precision for conversion */
> +static inline int w1_DS18B20_precision(struct device *device, int val);
> +static inline int w1_DS18S20_precision(struct device *device, int val);
> +
>  /* The return value is millidegrees Centigrade. */
>  static inline int w1_DS18B20_convert_temp(u8 rom[9]);
>  static inline int w1_DS18S20_convert_temp(u8 rom[9]);
> @@ -163,26 +175,194 @@ static inline int w1_DS18S20_convert_temp(u8 rom[9]);
>  static struct w1_therm_family_converter w1_therm_families[] = {
>  {
>  .f = _therm_family_DS18S20,
> - .convert = w1_DS18S20_convert_temp
> + .convert = w1_DS18S20_convert_temp,
> + .precision = w1_DS18S20_precision,
> + .eeprom = w1_therm_eeprom
>  },
>  {
>  .f = _therm_family_DS1822,
> - .convert = w1_DS18B20_convert_temp
> + .convert = w1_DS18B20_convert_temp,
> + .precision = w1_DS18S20_precision,
> + .eeprom = w1_therm_eeprom
>  },
>  {
>  .f = _therm_family_DS18B20,
> - .convert = w1_DS18B20_convert_temp
> + .convert = w1_DS18B20_convert_temp,
> + .precision = w1_DS18B20_precision,
> + .eeprom = w1_therm_eeprom
>  },
>  {
>  .f = _therm_family_DS28EA0

Re: [PATCH] w1: add ability to set (SRAM) and store (EEPROM) configuration for temp sensors like DS18B20

[Evgeniy Polyakov]

Hi

17.04.2016, 02:08, "x29a" <0.x29...@googlemail.com>:
> based on the PR (https://github.com/raspberrypi/linux/pull/1412) for the 
> raspberry pi kernel, i would like to propose my changes upstream as well.
>
> The changes entitle basic write operations for the 1Wire driver which are 
> needed to change e.g. the precision of temperature sensors like the very 
> popular DS18B20. Writing to SRAM and commiting the values to EEPROM is 
> possible.
>
> Since many sensors come "preconfigured" with a lower precision, people are 
> stuck that precision when running on a kernel based device (unlike the Dallas 
> 1Wire library for e.g. Arduino, which supports writing the 
> configuration/scratchpad).

This looks good to me, Greg, please pull it into your tree

> Signed-off-by: x29a <0.x29...@gmail.com>

Acked-by: Evgeniy Polyakov 

> ---
>  Documentation/w1/slaves/w1_therm | 10 +-
>  drivers/w1/slaves/w1_therm.c | 218 +--
>  drivers/w1/w1.h | 2 +
>  3 files changed, 222 insertions(+), 8 deletions(-)
>
> diff --git a/Documentation/w1/slaves/w1_therm 
> b/Documentation/w1/slaves/w1_therm
> index 13411fe..d1f93af 100644
> --- a/Documentation/w1/slaves/w1_therm
> +++ b/Documentation/w1/slaves/w1_therm
> @@ -33,7 +33,15 @@ temperature conversion at a time. If none of the devices 
> are parasite
>  powered it would be possible to convert all the devices at the same
>  time and then go back to read individual sensors. That isn't
>  currently supported. The driver also doesn't support reduced
> -precision (which would also reduce the conversion time).
> +precision (which would also reduce the conversion time) when reading values.
> +
> +Writing a value between 9 and 12 to the sysfs w1_slave file will change the
> +precision of the sensor for the next readings. This value is in (volatile)
> +SRAM, so it is reset when the sensor gets power-cycled.
> +
> +To store the current precision configuration into EEPROM, the value 0
> +has to be written to the sysfs w1_slave file. Since the EEPROM has a limited
> +amount of writes (>50k), this command should be used wisely.
>
>  The module parameter strong_pullup can be set to 0 to disable the
>  strong pullup, 1 to enable autodetection or 2 to force strong pullup.
> diff --git a/drivers/w1/slaves/w1_therm.c b/drivers/w1/slaves/w1_therm.c
> index 2f029e8..581a300 100644
> --- a/drivers/w1/slaves/w1_therm.c
> +++ b/drivers/w1/slaves/w1_therm.c
> @@ -92,10 +92,13 @@ static void w1_therm_remove_slave(struct w1_slave *sl)
>  static ssize_t w1_slave_show(struct device *device,
>  struct device_attribute *attr, char *buf);
>
> +static ssize_t w1_slave_store(struct device *device,
> + struct device_attribute *attr, const char *buf, size_t size);
> +
>  static ssize_t w1_seq_show(struct device *device,
>  struct device_attribute *attr, char *buf);
>
> -static DEVICE_ATTR_RO(w1_slave);
> +static DEVICE_ATTR_RW(w1_slave);
>  static DEVICE_ATTR_RO(w1_seq);
>
>  static struct attribute *w1_therm_attrs[] = {
> @@ -154,8 +157,17 @@ struct w1_therm_family_converter
>  u16 reserved;
>  struct w1_family *f;
>  int (*convert)(u8 rom[9]);
> + int (*precision)(struct device *device, int val);
> + int (*eeprom)(struct device *device);
>  };
>
> +/* write configuration to eeprom */
> +static inline int w1_therm_eeprom(struct device *device);
> +
> +/* Set precision for conversion */
> +static inline int w1_DS18B20_precision(struct device *device, int val);
> +static inline int w1_DS18S20_precision(struct device *device, int val);
> +
>  /* The return value is millidegrees Centigrade. */
>  static inline int w1_DS18B20_convert_temp(u8 rom[9]);
>  static inline int w1_DS18S20_convert_temp(u8 rom[9]);
> @@ -163,26 +175,194 @@ static inline int w1_DS18S20_convert_temp(u8 rom[9]);
>  static struct w1_therm_family_converter w1_therm_families[] = {
>  {
>  .f = _therm_family_DS18S20,
> - .convert = w1_DS18S20_convert_temp
> + .convert = w1_DS18S20_convert_temp,
> + .precision = w1_DS18S20_precision,
> + .eeprom = w1_therm_eeprom
>  },
>  {
>  .f = _therm_family_DS1822,
> - .convert = w1_DS18B20_convert_temp
> + .convert = w1_DS18B20_convert_temp,
> + .precision = w1_DS18S20_precision,
> + .eeprom = w1_therm_eeprom
>  },
>  {
>  .f = _therm_family_DS18B20,
> - .convert = w1_DS18B20_convert_temp
> + .convert = w1_DS18B20_convert_temp,
> + .precision = w1_DS18B20_precision,
> + .eeprom = w1_therm_eeprom
>  },
>  {
>  .f = _therm_family_DS28EA00,
> - .convert = w1_

Re: [patch] w1: silence an uninitialized variable warning

[Evgeniy Polyakov]

Hi Dan

14.04.2016, 12:36, "Dan Carpenter" :
> If kstrtoint() returns -ERANGE then "tmp" is uninitialized.

Looks good to me, please pull it into janitors tree

> Signed-off-by: Dan Carpenter 

Acked-by: Evgeniy Polaykov 

> diff --git a/drivers/w1/w1.c b/drivers/w1/w1.c
> index 89a7847..bb34362 100644
> --- a/drivers/w1/w1.c
> +++ b/drivers/w1/w1.c
> @@ -335,7 +335,7 @@ static ssize_t 
> w1_master_attribute_store_max_slave_count(struct device *dev,
>  int tmp;
>  struct w1_master *md = dev_to_w1_master(dev);
>
> - if (kstrtoint(buf, 0, ) == -EINVAL || tmp < 1)
> + if (kstrtoint(buf, 0, ) || tmp < 1)
>  return -EINVAL;
>
>  mutex_lock(>mutex);

Re: [patch] w1: silence an uninitialized variable warning

[Evgeniy Polyakov]

Hi Dan

14.04.2016, 12:36, "Dan Carpenter" :
> If kstrtoint() returns -ERANGE then "tmp" is uninitialized.

Looks good to me, please pull it into janitors tree

> Signed-off-by: Dan Carpenter 

Acked-by: Evgeniy Polaykov 

> diff --git a/drivers/w1/w1.c b/drivers/w1/w1.c
> index 89a7847..bb34362 100644
> --- a/drivers/w1/w1.c
> +++ b/drivers/w1/w1.c
> @@ -335,7 +335,7 @@ static ssize_t 
> w1_master_attribute_store_max_slave_count(struct device *dev,
>  int tmp;
>  struct w1_master *md = dev_to_w1_master(dev);
>
> - if (kstrtoint(buf, 0, ) == -EINVAL || tmp < 1)
> + if (kstrtoint(buf, 0, ) || tmp < 1)
>  return -EINVAL;
>
>  mutex_lock(>mutex);

Re: [PATCH] w1: w1_process() is not freezable kthread

[Evgeniy Polyakov]

Hi

28.10.2015, 08:26, "Jiri Kosina" :
> On Tue, 27 Oct 2015, Evgeniy Polyakov wrote:
>
>>  > w1_process() calls try_to_freeze(), but the thread doesn't mark itself
>>  > freezable through set_freezable(), so the try_to_freeze() call is useless.
>>
>>  I believe it is better to mark it freezable, what do you think? Its task
>>  is useless if anyone else goes sleeping, it should freeze too.
>
> I fail to see why this kthread should be freezable at all. There is no way
> for w1 device to generate new I/O requests that should be written out to
> filesystem, is it?

w1 doesn't generate such requests, but it was more to make this thread
consistent with majority of other threads in the kernel.

Ok, I'm not against it, Greg please pull this patch into your tree.

Acked-by: Evgeniy Polyakov 
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH] w1: w1_process() is not freezable kthread

[Evgeniy Polyakov]

Hi

28.10.2015, 08:26, "Jiri Kosina" <ji...@kernel.org>:
> On Tue, 27 Oct 2015, Evgeniy Polyakov wrote:
>
>>  > w1_process() calls try_to_freeze(), but the thread doesn't mark itself
>>  > freezable through set_freezable(), so the try_to_freeze() call is useless.
>>
>>  I believe it is better to mark it freezable, what do you think? Its task
>>  is useless if anyone else goes sleeping, it should freeze too.
>
> I fail to see why this kthread should be freezable at all. There is no way
> for w1 device to generate new I/O requests that should be written out to
> filesystem, is it?

w1 doesn't generate such requests, but it was more to make this thread
consistent with majority of other threads in the kernel.

Ok, I'm not against it, Greg please pull this patch into your tree.

Acked-by: Evgeniy Polyakov <z...@ioremap.net>
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH] w1: w1_process() is not freezable kthread

[Evgeniy Polyakov]

Hi

26.10.2015, 08:53, "Jiri Kosina" :
> From: Jiri Kosina 
>
> w1_process() calls try_to_freeze(), but the thread doesn't mark itself
> freezable through set_freezable(), so the try_to_freeze() call is useless.

I believe it is better to mark it freezable, what do you think?
Its task is useless if anyone else goes sleeping, it should freeze too.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH] w1: w1_process() is not freezable kthread

[Evgeniy Polyakov]

Hi

26.10.2015, 08:53, "Jiri Kosina" :
> From: Jiri Kosina 
>
> w1_process() calls try_to_freeze(), but the thread doesn't mark itself
> freezable through set_freezable(), so the try_to_freeze() call is useless.

I believe it is better to mark it freezable, what do you think?
Its task is useless if anyone else goes sleeping, it should freeze too.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH] w1: Use module_pci_driver

[Evgeniy Polyakov]

Hi everyone

Patch looks good to me, thank you
Greg, please pull it into your tree

Acked-by: Evgeniy Polyakov 

07.07.2015, 09:55, "Vaishali Thakkar" :
> Use module_pci_driver for drivers whose init and exit functions
> only register and unregister, respectively.

> Signed-off-by: Vaishali Thakkar 
> ---
>  drivers/w1/masters/matrox_w1.c | 14 +-
>  1 file changed, 1 insertion(+), 13 deletions(-)
>
> diff --git a/drivers/w1/masters/matrox_w1.c b/drivers/w1/masters/matrox_w1.c
> index d8667b0..684bc9d 100644
> --- a/drivers/w1/masters/matrox_w1.c
> +++ b/drivers/w1/masters/matrox_w1.c
> @@ -232,16 +232,4 @@ static void matrox_w1_remove(struct pci_dev *pdev)
>  }
>  kfree(dev);
>  }
> -
> -static int __init matrox_w1_init(void)
> -{
> - return pci_register_driver(_w1_pci_driver);
> -}
> -
> -static void __exit matrox_w1_fini(void)
> -{
> - pci_unregister_driver(_w1_pci_driver);
> -}
> -
> -module_init(matrox_w1_init);
> -module_exit(matrox_w1_fini);
> +module_pci_driver(matrox_w1_pci_driver);
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH] w1: Drop owner assignment from i2c_driver

[Evgeniy Polyakov]

Hi Krzysztof

Patch looks good to me, thank you.
Greg, please pull it into your tree.

Acked-by: Evgeniy Polyakov 

10.07.2015, 09:38, "Krzysztof Kozlowski" :
> i2c_driver does not need to set an owner because i2c_register_driver()
> will set it.
>
> Signed-off-by: Krzysztof Kozlowski 
>
> ---
>
> The coccinelle script which generated the patch was sent here:
> http://www.spinics.net/lists/kernel/msg2029903.html
> ---
>  drivers/w1/masters/ds2482.c | 1 -
>  1 file changed, 1 deletion(-)
>
> diff --git a/drivers/w1/masters/ds2482.c b/drivers/w1/masters/ds2482.c
> index a674409edfb3..b05e8fefbabd 100644
> --- a/drivers/w1/masters/ds2482.c
> +++ b/drivers/w1/masters/ds2482.c
> @@ -97,7 +97,6 @@ MODULE_DEVICE_TABLE(i2c, ds2482_id);
>
>  static struct i2c_driver ds2482_driver = {
>  .driver = {
> - .owner = THIS_MODULE,
>  .name = "ds2482",
>  },
>  .probe = ds2482_probe,
> --
> 1.9.1
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH] w1: Drop owner assignment from i2c_driver

[Evgeniy Polyakov]

Hi Krzysztof

Patch looks good to me, thank you.
Greg, please pull it into your tree.

Acked-by: Evgeniy Polyakov z...@ioremap.net

10.07.2015, 09:38, Krzysztof Kozlowski k.kozlow...@samsung.com:
 i2c_driver does not need to set an owner because i2c_register_driver()
 will set it.

 Signed-off-by: Krzysztof Kozlowski k.kozlow...@samsung.com

 ---

 The coccinelle script which generated the patch was sent here:
 http://www.spinics.net/lists/kernel/msg2029903.html
 ---
  drivers/w1/masters/ds2482.c | 1 -
  1 file changed, 1 deletion(-)

 diff --git a/drivers/w1/masters/ds2482.c b/drivers/w1/masters/ds2482.c
 index a674409edfb3..b05e8fefbabd 100644
 --- a/drivers/w1/masters/ds2482.c
 +++ b/drivers/w1/masters/ds2482.c
 @@ -97,7 +97,6 @@ MODULE_DEVICE_TABLE(i2c, ds2482_id);

  static struct i2c_driver ds2482_driver = {
  .driver = {
 - .owner = THIS_MODULE,
  .name = ds2482,
  },
  .probe = ds2482_probe,
 --
 1.9.1
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH] w1: Use module_pci_driver

[Evgeniy Polyakov]

Hi everyone

Patch looks good to me, thank you
Greg, please pull it into your tree

Acked-by: Evgeniy Polyakov z...@ioremap.net

07.07.2015, 09:55, Vaishali Thakkar vthakkar1...@gmail.com:
 Use module_pci_driver for drivers whose init and exit functions
 only register and unregister, respectively.

 Signed-off-by: Vaishali Thakkar vthakkar1...@gmail.com
 ---
  drivers/w1/masters/matrox_w1.c | 14 +-
  1 file changed, 1 insertion(+), 13 deletions(-)

 diff --git a/drivers/w1/masters/matrox_w1.c b/drivers/w1/masters/matrox_w1.c
 index d8667b0..684bc9d 100644
 --- a/drivers/w1/masters/matrox_w1.c
 +++ b/drivers/w1/masters/matrox_w1.c
 @@ -232,16 +232,4 @@ static void matrox_w1_remove(struct pci_dev *pdev)
  }
  kfree(dev);
  }
 -
 -static int __init matrox_w1_init(void)
 -{
 - return pci_register_driver(matrox_w1_pci_driver);
 -}
 -
 -static void __exit matrox_w1_fini(void)
 -{
 - pci_unregister_driver(matrox_w1_pci_driver);
 -}
 -
 -module_init(matrox_w1_init);
 -module_exit(matrox_w1_fini);
 +module_pci_driver(matrox_w1_pci_driver);
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [patch] [patch 2/2 v2] w1: use correct lock on error in w1_seq_show()

[Evgeniy Polyakov]

Hi

04.06.2015, 12:04, "Dan Carpenter" :
> I noticed there was a problem here because Smatch complained:
>
> drivers/w1/slaves/w1_therm.c:416 w1_seq_show() warn:
> inconsistent returns 'mutex:>master->mutex'.
>   Locked on: line 416
>   Unlocked on: line 413
>
> The problem is that we lock ->mutex but we unlock ->bus_mutex on error.
> David Fries says that ->bus_mutex is correct and ->mutex is incorrect.
>
> Fixes: d9411e57dc7f ('w1: Add support for DS28EA00 sequence to w1-therm')
> Signed-off-by: Dan Carpenter 

Looks good to me, Greg please pull this serie into your tree, if you hadn't yet.
Am I right that this is a stable tree material too?

Acked-by: Evgeniy Polyakov 
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH v2] w1: masters: omap_hdq: Add support for 1-wire mode

[Evgeniy Polyakov]

Hi

25.05.2015, 08:15, "Vignesh R" :
>>  HDQ mode remains unchanged.
>>
>>  Signed-off-by: Vignesh R 

I have no experience with omap_hdq platform, but there are quite a few questions
related to IO - you never check whether write was successful or read returned 
actually
valid data, is it ok? I mean is it correct to assume that read can not return 
0xff for example
and it is a sign that something is wrong, or this can not happen?

As for me, I have no objection, but this patch must go via omap tree imo.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH v2] w1: masters: omap_hdq: Add support for 1-wire mode

[Evgeniy Polyakov]

Hi

25.05.2015, 08:15, Vignesh R vigne...@ti.com:
  HDQ mode remains unchanged.

  Signed-off-by: Vignesh R vigne...@ti.com

I have no experience with omap_hdq platform, but there are quite a few questions
related to IO - you never check whether write was successful or read returned 
actually
valid data, is it ok? I mean is it correct to assume that read can not return 
0xff for example
and it is a sign that something is wrong, or this can not happen?

As for me, I have no objection, but this patch must go via omap tree imo.
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [patch] [patch 2/2 v2] w1: use correct lock on error in w1_seq_show()

[Evgeniy Polyakov]

Hi

04.06.2015, 12:04, Dan Carpenter dan.carpen...@oracle.com:
 I noticed there was a problem here because Smatch complained:

 drivers/w1/slaves/w1_therm.c:416 w1_seq_show() warn:
 inconsistent returns 'mutex:sl-master-mutex'.
   Locked on: line 416
   Unlocked on: line 413

 The problem is that we lock -mutex but we unlock -bus_mutex on error.
 David Fries says that -bus_mutex is correct and -mutex is incorrect.

 Fixes: d9411e57dc7f ('w1: Add support for DS28EA00 sequence to w1-therm')
 Signed-off-by: Dan Carpenter dan.carpen...@oracle.com

Looks good to me, Greg please pull this serie into your tree, if you hadn't yet.
Am I right that this is a stable tree material too?

Acked-by: Evgeniy Polyakov z...@ioremap.net
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/