netdev.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 00/20] world-writable files in sysfs and debugfs
@ 2011-02-04 12:22 Vasiliy Kulikov
  2011-02-04 12:23 ` [PATCH 12/20] net: can: at91_can: world-writable sysfs files Vasiliy Kulikov
                   ` (2 more replies)
  0 siblings, 3 replies; 15+ messages in thread
From: Vasiliy Kulikov @ 2011-02-04 12:22 UTC (permalink / raw)
  To: linux-kernel
  Cc: Mike Christie, Srinidhi Kasagar, Tony Lindgren,
	platform-driver-x86, socketcan-core, Corentin Chary,
	James E.J. Bottomley, Julia Lawall, Russell King, Samuel Ortiz,
	linux-scsi, Karol Kozimor, Kevin Hilman, Luca Risolia, open-iscsi,
	Wolfgang Grandegger, Matthew Garrett, acpi4asus-user, rtc-linux,
	Carlos Corbacho, Mauro Carvalho Chehab, linux-omap,
	linux-arm-kernel

The search was made with trivial shell commands:

find | xargs grep S_IWUGO
find | xargs grep S_IWOTH

I didn't precisely investigate how exactly one may damage the
system/hardware because of issues number, maybe the harm is very limited
in case of some of these drivers.

One suspicious file is ./staging/speakup/speakup.h, but it explitly calls
macros as world-writable.  I didn't check what speakup's world-writable
files provide because it requires some knowledge about the hardware.


Vasiliy Kulikov (20):
  mach-omap2: mux: world-writable debugfs files
  mach-omap2: pm: world-writable debugfs timer files
  mach-omap2: smartreflex: world-writable debugfs voltage files
  mach-ux500: mbox-db5500: world-writable sysfs fifo file
  leds: lp5521: world-writable sysfs engine* files
  leds: lp5523: world-writable engine* sysfs files
  video: sn9c102: world-wirtable sysfs files
  mfd: ab3100: world-writable debugfs *_priv files
  mfd: ab3500: world-writable debugfs register-* files
  mfd: ab8500: world-writable debugfs register-* files
  misc: ep93xx_pwm: world-writable sysfs files
  net: can: at91_can: world-writable sysfs files
  net: can: janz-ican3: world-writable sysfs termination file
  platform: x86: acer-wmi: world-writable sysfs threeg file
  platform: x86: asus_acpi: world-writable procfs files
  platform: x86: tc1100-wmi: world-writable sysfs wireless and jogdial files
  rtc: rtc-ds1511: world-writable sysfs nvram file
  scsi: aic94xx: world-writable sysfs update_bios file
  scsi: iscsi: world-writable sysfs priv_sess file
  fs: ubifs: world-writable debugfs dump_* files

 arch/arm/mach-omap2/mux.c                  |    2 +-
 arch/arm/mach-omap2/pm-debug.c             |    8 ++++----
 arch/arm/mach-omap2/smartreflex.c          |    4 ++--
 arch/arm/mach-ux500/mbox-db5500.c          |    2 +-
 drivers/leds/leds-lp5521.c                 |   14 +++++++-------
 drivers/leds/leds-lp5523.c                 |   20 ++++++++++----------
 drivers/media/video/sn9c102/sn9c102_core.c |    6 +++---
 drivers/mfd/ab3100-core.c                  |    4 ++--
 drivers/mfd/ab3550-core.c                  |    6 +++---
 drivers/mfd/ab8500-debugfs.c               |    6 +++---
 drivers/misc/ep93xx_pwm.c                  |    6 +++---
 drivers/net/can/at91_can.c                 |    2 +-
 drivers/net/can/janz-ican3.c               |    2 +-
 drivers/platform/x86/acer-wmi.c            |    2 +-
 drivers/platform/x86/asus_acpi.c           |    8 +-------
 drivers/platform/x86/tc1100-wmi.c          |    2 +-
 drivers/rtc/rtc-ds1511.c                   |    2 +-
 drivers/scsi/aic94xx/aic94xx_init.c        |    2 +-
 drivers/scsi/scsi_transport_iscsi.c        |    2 +-
 fs/ubifs/debug.c                           |    6 +++---
 20 files changed, 50 insertions(+), 56 deletions(-)

--
Vasiliy Kulikov
http://www.openwall.com - bringing security into open computing environments

^ permalink raw reply	[flat|nested] 15+ messages in thread

* [PATCH 12/20] net: can: at91_can: world-writable sysfs files
  2011-02-04 12:22 [PATCH 00/20] world-writable files in sysfs and debugfs Vasiliy Kulikov
@ 2011-02-04 12:23 ` Vasiliy Kulikov
       [not found]   ` <a6800dc8b0daed78256f98f52844cbbb48f4a76d.1296818921.git.segoon-cxoSlKxDwOJWk0Htik3J/w@public.gmane.org>
  2011-02-04 12:23 ` [PATCH 13/20] net: can: janz-ican3: world-writable sysfs termination file Vasiliy Kulikov
  2011-02-04 13:11 ` [rtc-linux] [PATCH 00/20] world-writable files in sysfs and debugfs Linus Walleij
  2 siblings, 1 reply; 15+ messages in thread
From: Vasiliy Kulikov @ 2011-02-04 12:23 UTC (permalink / raw)
  To: linux-kernel; +Cc: security, Wolfgang Grandegger, socketcan-core, netdev

Don't allow everybody to write to mb0_id file.

Signed-off-by: Vasiliy Kulikov <segoon@openwall.com>
---
 Cannot compile the driver, so it is not tested at all.

 drivers/net/can/at91_can.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/drivers/net/can/at91_can.c b/drivers/net/can/at91_can.c
index 2532b96..57d2ffb 100644
--- a/drivers/net/can/at91_can.c
+++ b/drivers/net/can/at91_can.c
@@ -1109,7 +1109,7 @@ static ssize_t at91_sysfs_set_mb0_id(struct device *dev,
 	return ret;
 }
 
-static DEVICE_ATTR(mb0_id, S_IWUGO | S_IRUGO,
+static DEVICE_ATTR(mb0_id, S_IWUSR | S_IRUGO,
 	at91_sysfs_show_mb0_id, at91_sysfs_set_mb0_id);
 
 static struct attribute *at91_sysfs_attrs[] = {
-- 
1.7.0.4

^ permalink raw reply related	[flat|nested] 15+ messages in thread

* [PATCH 13/20] net: can: janz-ican3: world-writable sysfs termination file
  2011-02-04 12:22 [PATCH 00/20] world-writable files in sysfs and debugfs Vasiliy Kulikov
  2011-02-04 12:23 ` [PATCH 12/20] net: can: at91_can: world-writable sysfs files Vasiliy Kulikov
@ 2011-02-04 12:23 ` Vasiliy Kulikov
       [not found]   ` <6b49b9521416fbd50214485d3e14e5f254ada4f7.1296818921.git.segoon-cxoSlKxDwOJWk0Htik3J/w@public.gmane.org>
  2011-02-04 13:11 ` [rtc-linux] [PATCH 00/20] world-writable files in sysfs and debugfs Linus Walleij
  2 siblings, 1 reply; 15+ messages in thread
From: Vasiliy Kulikov @ 2011-02-04 12:23 UTC (permalink / raw)
  To: linux-kernel; +Cc: security, Wolfgang Grandegger, socketcan-core, netdev

Don't allow everybody to set terminator via sysfs.

Signed-off-by: Vasiliy Kulikov <segoon@openwall.com>
---
 Compile tested only.

 drivers/net/can/janz-ican3.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/drivers/net/can/janz-ican3.c b/drivers/net/can/janz-ican3.c
index b9a6d7a..366f5cc 100644
--- a/drivers/net/can/janz-ican3.c
+++ b/drivers/net/can/janz-ican3.c
@@ -1618,7 +1618,7 @@ static ssize_t ican3_sysfs_set_term(struct device *dev,
 	return count;
 }
 
-static DEVICE_ATTR(termination, S_IWUGO | S_IRUGO, ican3_sysfs_show_term,
+static DEVICE_ATTR(termination, S_IWUSR | S_IRUGO, ican3_sysfs_show_term,
 						   ican3_sysfs_set_term);
 
 static struct attribute *ican3_sysfs_attrs[] = {
-- 
1.7.0.4

^ permalink raw reply related	[flat|nested] 15+ messages in thread

* Re: [PATCH 12/20] net: can: at91_can: world-writable sysfs files
       [not found]   ` <a6800dc8b0daed78256f98f52844cbbb48f4a76d.1296818921.git.segoon-cxoSlKxDwOJWk0Htik3J/w@public.gmane.org>
@ 2011-02-04 12:42     ` Kurt Van Dijck
       [not found]       ` <20110204124233.GB334-MxZ6Iy/zr/UdbCeoMzGj59i2O/JbrIOy@public.gmane.org>
  0 siblings, 1 reply; 15+ messages in thread
From: Kurt Van Dijck @ 2011-02-04 12:42 UTC (permalink / raw)
  To: Vasiliy Kulikov
  Cc: socketcan-core-0fE9KPoRgkgATYTw5x5z8w,
	netdev-u79uwXL29TY76Z2rM5mHXA, security-DgEjT+Ai2ygdnm+yROfE0A,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA, Wolfgang Grandegger

On Fri, Feb 04, 2011 at 03:23:50PM +0300, Vasiliy Kulikov wrote:
> Don't allow everybody to write to mb0_id file.
> 
very well!

Acked-by: Kurt Van Dijck <kurt.van.dijck-/BeEPy95v10@public.gmane.org>

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [rtc-linux] [PATCH 00/20] world-writable files in sysfs and debugfs
  2011-02-04 12:22 [PATCH 00/20] world-writable files in sysfs and debugfs Vasiliy Kulikov
  2011-02-04 12:23 ` [PATCH 12/20] net: can: at91_can: world-writable sysfs files Vasiliy Kulikov
  2011-02-04 12:23 ` [PATCH 13/20] net: can: janz-ican3: world-writable sysfs termination file Vasiliy Kulikov
@ 2011-02-04 13:11 ` Linus Walleij
  2 siblings, 0 replies; 15+ messages in thread
From: Linus Walleij @ 2011-02-04 13:11 UTC (permalink / raw)
  To: rtc-linux
  Cc: Mike Christie, Srinidhi Kasagar, Tony Lindgren,
	platform-driver-x86, socketcan-core, Corentin Chary,
	James E.J. Bottomley, Julia Lawall, Russell King, Samuel Ortiz,
	linux-scsi, Karol Kozimor, Kevin Hilman, Luca Risolia, open-iscsi,
	Wolfgang Grandegger, Matthew Garrett, acpi4asus-user,
	Carlos Corbacho, Mauro Carvalho Chehab, linux-omap,
	linux-arm-kernel, Alessandro Zummo <a.zummo@

2011/2/4 Vasiliy Kulikov <segoon@openwall.com>:

> The search was made with trivial shell commands:
>
> find | xargs grep S_IWUGO
> find | xargs grep S_IWOTH

We only use our debugfs entries as root so it shouldn't matter much, this
is way better, thanks for fixing.

>  mach-ux500: mbox-db5500: world-writable sysfs fifo file
>  mfd: ab3100: world-writable debugfs *_priv files
>  mfd: ab3500: world-writable debugfs register-* files
>  mfd: ab8500: world-writable debugfs register-* files

Acked-by: Linus Walleij <linus.walleij@stericsson.com>

For these.

Yours,
Linus Walleij

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH 13/20] net: can: janz-ican3: world-writable sysfs termination file
       [not found]   ` <6b49b9521416fbd50214485d3e14e5f254ada4f7.1296818921.git.segoon-cxoSlKxDwOJWk0Htik3J/w@public.gmane.org>
@ 2011-02-04 21:06     ` David Miller
  0 siblings, 0 replies; 15+ messages in thread
From: David Miller @ 2011-02-04 21:06 UTC (permalink / raw)
  To: segoon-cxoSlKxDwOJWk0Htik3J/w
  Cc: socketcan-core-0fE9KPoRgkgATYTw5x5z8w,
	netdev-u79uwXL29TY76Z2rM5mHXA, security-DgEjT+Ai2ygdnm+yROfE0A,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA, wg-5Yr1BZd7O62+XT7JhA+gdA

From: Vasiliy Kulikov <segoon-cxoSlKxDwOJWk0Htik3J/w@public.gmane.org>
Date: Fri,  4 Feb 2011 15:23:53 +0300

> Don't allow everybody to set terminator via sysfs.
> 
> Signed-off-by: Vasiliy Kulikov <segoon-cxoSlKxDwOJWk0Htik3J/w@public.gmane.org>

Applied.

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH 12/20] net: can: at91_can: world-writable sysfs files
       [not found]       ` <20110204124233.GB334-MxZ6Iy/zr/UdbCeoMzGj59i2O/JbrIOy@public.gmane.org>
@ 2011-02-04 21:06         ` David Miller
  2011-02-07 11:38           ` About bittiming calculation result Tomoya MORINAGA
  0 siblings, 1 reply; 15+ messages in thread
From: David Miller @ 2011-02-04 21:06 UTC (permalink / raw)
  To: kurt.van.dijck-/BeEPy95v10
  Cc: security-DgEjT+Ai2ygdnm+yROfE0A, netdev-u79uwXL29TY76Z2rM5mHXA,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA,
	socketcan-core-0fE9KPoRgkgATYTw5x5z8w,
	segoon-cxoSlKxDwOJWk0Htik3J/w, wg-5Yr1BZd7O62+XT7JhA+gdA

From: Kurt Van Dijck <kurt.van.dijck-/BeEPy95v10@public.gmane.org>
Date: Fri, 4 Feb 2011 13:42:33 +0100

> On Fri, Feb 04, 2011 at 03:23:50PM +0300, Vasiliy Kulikov wrote:
>> Don't allow everybody to write to mb0_id file.
>> 
> very well!
> 
> Acked-by: Kurt Van Dijck <kurt.van.dijck-/BeEPy95v10@public.gmane.org>

Applied.

^ permalink raw reply	[flat|nested] 15+ messages in thread

* About bittiming calculation result
  2011-02-04 21:06         ` David Miller
@ 2011-02-07 11:38           ` Tomoya MORINAGA
       [not found]             ` <5009516791F146C49C73FAC57C437313-c0cKtqp5df7I9507bXv2FdBPR1lH4CV8@public.gmane.org>
  0 siblings, 1 reply; 15+ messages in thread
From: Tomoya MORINAGA @ 2011-02-07 11:38 UTC (permalink / raw)
  To: wg, socketcan-core; +Cc: netdev, linux-kernel

Hi,

I have a question for bittiming-value calculated by Can-core.

In case setting like below,
 - ip link set can0 type can bitrate 800000
 - clock=50MHz
 - Use pch_can

Can-core calculates like below
brp=21
seg1=1
seg2=1
sjw=1
prop_seg=0

Is "prop_seg=0" true ?
seg1/seg2/sjw/prop_seg must be more than 1 ?

Also I can see the following kernel error log.
bitrate error 0.7%

Thanks,
-----------------------------------------
Tomoya MORINAGA
OKI SEMICONDUCTOR CO., LTD.

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: About bittiming calculation result
       [not found]             ` <5009516791F146C49C73FAC57C437313-c0cKtqp5df7I9507bXv2FdBPR1lH4CV8@public.gmane.org>
@ 2011-02-07 12:00               ` Wolfgang Grandegger
       [not found]                 ` <4D4FDEF9.2030305-5Yr1BZd7O62+XT7JhA+gdA@public.gmane.org>
  0 siblings, 1 reply; 15+ messages in thread
From: Wolfgang Grandegger @ 2011-02-07 12:00 UTC (permalink / raw)
  To: Tomoya MORINAGA
  Cc: socketcan-core-0fE9KPoRgkgATYTw5x5z8w,
	netdev-u79uwXL29TY76Z2rM5mHXA,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA

Hi Tomoya,

On 02/07/2011 12:38 PM, Tomoya MORINAGA wrote:
> Hi,
> 
> I have a question for bittiming-value calculated by Can-core.
> 
> In case setting like below,
>  - ip link set can0 type can bitrate 800000
>  - clock=50MHz
>  - Use pch_can
> 
> Can-core calculates like below
> brp=21
> seg1=1
> seg2=1
> sjw=1
> prop_seg=0
> 
> Is "prop_seg=0" true ?

Well, only prop_seg+phase_seg=tseg1 is relevant and the pch_can driver
sets the allowed minimum "tseg1_min1" currently to 1:

static struct can_bittiming_const pch_can_bittiming_const = {
        .name = KBUILD_MODNAME,
        .tseg1_min = 1,
        .tseg1_max = 16,
        .tseg2_min = 1,
        .tseg2_max = 8,
        .sjw_max = 4,
        .brp_min = 1,
        .brp_max = 1024, /* 6bit + extended 4bit */
        .brp_inc = 1,
};

> seg1/seg2/sjw/prop_seg must be more than 1 ?

Then "tseg1_min" should be set to *2*.

> Also I can see the following kernel error log.
> bitrate error 0.7%

A clock frequency of 50 MHz is sub-optimal for CAN and some
bit-rates cannot be reproduced properly. Here is the output of
the can-utils program "can-calc-bit-timing" (with an entry for
the pch-can added):

$ ./can-calc-bit-timing pch-can
Bit timing parameters for pch-can with 50.000000 MHz ref clock
nominal                                 real Bitrt   nom  real SampP
Bitrate TQ[ns] PrS PhS1 PhS2 SJW BRP Bitrate Error SampP SampP Error CNF1 CNF2 CNF3
1000000    100   3    3    3   1   5 1000000  0.0% 75.0% 70.0%  6.7% 0x05 0x92 0x02
 800000    420   0    1    1   1  21  793650  0.8% 80.0% 66.6% 16.8% 0x15 0xff 0x00
 500000    100   8    8    3   1   5  500000  0.0% 87.5% 85.0%  2.9% 0x05 0xbf 0x02
 250000    500   3    3    1   1  25  250000  0.0% 87.5% 87.5%  0.0% 0x19 0x92 0x00
 125000    500   6    7    2   1  25  125000  0.0% 87.5% 87.5%  0.0% 0x19 0xb5 0x01
 100000    500   8    8    3   1  25  100000  0.0% 87.5% 85.0%  2.9% 0x19 0xbf 0x02
  50000   2500   3    3    1   1 125   50000  0.0% 87.5% 87.5%  0.0% 0x7d 0x92 0x00
  20000   2500   8    8    3   1 125   20000  0.0% 87.5% 85.0%  2.9% 0x7d 0xbf 0x02
  10000  12500   3    3    1   1 625   10000  0.0% 87.5% 87.5%  0.0% 0x71 0x92 0x00

As you can see, especially 800000 gives rather bad results.

Wolfgang.

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: About bittiming calculation result
       [not found]                 ` <4D4FDEF9.2030305-5Yr1BZd7O62+XT7JhA+gdA@public.gmane.org>
@ 2011-02-07 15:52                   ` Wolfgang Grandegger
       [not found]                     ` <4D501555.5000905-5Yr1BZd7O62+XT7JhA+gdA@public.gmane.org>
  2011-02-08  1:09                   ` Tomoya MORINAGA
  1 sibling, 1 reply; 15+ messages in thread
From: Wolfgang Grandegger @ 2011-02-07 15:52 UTC (permalink / raw)
  To: Tomoya MORINAGA
  Cc: socketcan-core-0fE9KPoRgkgATYTw5x5z8w,
	netdev-u79uwXL29TY76Z2rM5mHXA,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA

On 02/07/2011 01:00 PM, Wolfgang Grandegger wrote:
> Hi Tomoya,
> 
> On 02/07/2011 12:38 PM, Tomoya MORINAGA wrote:
>> Hi,
>>
>> I have a question for bittiming-value calculated by Can-core.
>>
>> In case setting like below,
>>  - ip link set can0 type can bitrate 800000
>>  - clock=50MHz
>>  - Use pch_can
>>
>> Can-core calculates like below
>> brp=21
>> seg1=1
>> seg2=1
>> sjw=1
>> prop_seg=0
>>
>> Is "prop_seg=0" true ?
> 
> Well, only prop_seg+phase_seg=tseg1 is relevant and the pch_can driver
> sets the allowed minimum "tseg1_min1" currently to 1:
> 
> static struct can_bittiming_const pch_can_bittiming_const = {
>         .name = KBUILD_MODNAME,
>         .tseg1_min = 1,
>         .tseg1_max = 16,
>         .tseg2_min = 1,
>         .tseg2_max = 8,
>         .sjw_max = 4,
>         .brp_min = 1,
>         .brp_max = 1024, /* 6bit + extended 4bit */
>         .brp_inc = 1,
> };
> 
>> seg1/seg2/sjw/prop_seg must be more than 1 ?
> 
> Then "tseg1_min" should be set to *2*.
> 
>> Also I can see the following kernel error log.
>> bitrate error 0.7%
> 
> A clock frequency of 50 MHz is sub-optimal for CAN and some
> bit-rates cannot be reproduced properly. Here is the output of
> the can-utils program "can-calc-bit-timing" (with an entry for
> the pch-can added):
> 
> $ ./can-calc-bit-timing pch-can
> Bit timing parameters for pch-can with 50.000000 MHz ref clock
> nominal                                 real Bitrt   nom  real SampP
> Bitrate TQ[ns] PrS PhS1 PhS2 SJW BRP Bitrate Error SampP SampP Error CNF1 CNF2 CNF3
> 1000000    100   3    3    3   1   5 1000000  0.0% 75.0% 70.0%  6.7% 0x05 0x92 0x02
>  800000    420   0    1    1   1  21  793650  0.8% 80.0% 66.6% 16.8% 0x15 0xff 0x00
>  500000    100   8    8    3   1   5  500000  0.0% 87.5% 85.0%  2.9% 0x05 0xbf 0x02
>  250000    500   3    3    1   1  25  250000  0.0% 87.5% 87.5%  0.0% 0x19 0x92 0x00
>  125000    500   6    7    2   1  25  125000  0.0% 87.5% 87.5%  0.0% 0x19 0xb5 0x01
>  100000    500   8    8    3   1  25  100000  0.0% 87.5% 85.0%  2.9% 0x19 0xbf 0x02
>   50000   2500   3    3    1   1 125   50000  0.0% 87.5% 87.5%  0.0% 0x7d 0x92 0x00
>   20000   2500   8    8    3   1 125   20000  0.0% 87.5% 85.0%  2.9% 0x7d 0xbf 0x02
>   10000  12500   3    3    1   1 625   10000  0.0% 87.5% 87.5%  0.0% 0x71 0x92 0x00
> 
> As you can see, especially 800000 gives rather bad results.

BTW, it's always possible to specify optimized bit-timing parameters
directly, e.g. the following seem better:

   800000     60  12    4    4   4   3  793650  0.8% 80.0% 81.0%  1.2%

You could set these with:

  $ ip link set can0 type can \
    tq 60 prop-seg 12 phase-seg1 4 phase-seg2 4 sjw 4

Wolfgang.

^ permalink raw reply	[flat|nested] 15+ messages in thread

* RE: About bittiming calculation result
       [not found]                 ` <4D4FDEF9.2030305-5Yr1BZd7O62+XT7JhA+gdA@public.gmane.org>
  2011-02-07 15:52                   ` Wolfgang Grandegger
@ 2011-02-08  1:09                   ` Tomoya MORINAGA
       [not found]                     ` <E2BAACFF191C4175854E6B2EB9135BE5-c0cKtqp5df7I9507bXv2FdBPR1lH4CV8@public.gmane.org>
  1 sibling, 1 reply; 15+ messages in thread
From: Tomoya MORINAGA @ 2011-02-08  1:09 UTC (permalink / raw)
  To: 'Wolfgang Grandegger'
  Cc: socketcan-core-0fE9KPoRgkgATYTw5x5z8w,
	netdev-u79uwXL29TY76Z2rM5mHXA,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA

On Monday, February 07, 2011 9:01 PM, Wolfgang Grandegger wrote
> 
> Well, only prop_seg+phase_seg=tseg1 is relevant and the 
> pch_can driver sets the allowed minimum "tseg1_min1" currently to 1:
> 
> static struct can_bittiming_const pch_can_bittiming_const = {
>         .name = KBUILD_MODNAME,
>         .tseg1_min = 1,
>         .tseg1_max = 16,
>         .tseg2_min = 1,
>         .tseg2_max = 8,
>         .sjw_max = 4,
>         .brp_min = 1,
>         .brp_max = 1024, /* 6bit + extended 4bit */
>         .brp_inc = 1,
> };
> 
> > seg1/seg2/sjw/prop_seg must be more than 1 ?
> 
> Then "tseg1_min" should be set to *2*.

Though some drivers accepted by upstream have parameter "tseg1_min" as 1,
Sould we release the patch like below ?
 -         .tseg1_min = 1,
+         .tseg1_min = 2,

Thanks,
-----------------------------------------
Tomoya MORINAGA
OKI SEMICONDUCTOR CO., LTD.

^ permalink raw reply	[flat|nested] 15+ messages in thread

* RE: About bittiming calculation result
       [not found]                     ` <4D501555.5000905-5Yr1BZd7O62+XT7JhA+gdA@public.gmane.org>
@ 2011-02-08  1:27                       ` Tomoya MORINAGA
       [not found]                         ` <93C12206407640199DCDD3A89A333F13-c0cKtqp5df7I9507bXv2FdBPR1lH4CV8@public.gmane.org>
  0 siblings, 1 reply; 15+ messages in thread
From: Tomoya MORINAGA @ 2011-02-08  1:27 UTC (permalink / raw)
  To: 'Wolfgang Grandegger'
  Cc: socketcan-core-0fE9KPoRgkgATYTw5x5z8w,
	netdev-u79uwXL29TY76Z2rM5mHXA,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA

On Tuesday, February 08, 2011 12:53 AM,  Wolfgang Grandegger wrote:

> BTW, it's always possible to specify optimized bit-timing 
> parameters directly, e.g. the following seem better:
> 
>    800000     60  12    4    4   4   3  793650  0.8% 80.0% 81.0%  1.2%
> 
> You could set these with:
> 
>   $ ip link set can0 type can \
>     tq 60 prop-seg 12 phase-seg1 4 phase-seg2 4 sjw 4

I can confirm 800K comms works well using the above.

I wish Can-core could calculate like above.

>> seg1/seg2/sjw/prop_seg must be more than 1 ?
BTW, according to EG20T PCH data sheet, 
CAN bit-timing parameters(BRP, Prop_Seg, Phase_Seg1, Phase_Seg2, SJW) must not be set 0.

Thanks,
-----------------------------------------
Tomoya MORINAGA
OKI SEMICONDUCTOR CO., LTD.


> -----Original Message-----
> From: Wolfgang Grandegger [mailto:wg-5Yr1BZd7O62+XT7JhA+gdA@public.gmane.org] 
> Sent: Tuesday, February 08, 2011 12:53 AM
> To: Tomoya MORINAGA
> Cc: socketcan-core-0fE9KPoRgkgATYTw5x5z8w@public.gmane.org; netdev-u79uwXL29TY76Z2rM5mHXA@public.gmane.org; 
> linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
> Subject: Re: About bittiming calculation result
> 
> On 02/07/2011 01:00 PM, Wolfgang Grandegger wrote:
> > Hi Tomoya,
> > 
> > On 02/07/2011 12:38 PM, Tomoya MORINAGA wrote:
> >> Hi,
> >>
> >> I have a question for bittiming-value calculated by Can-core.
> >>
> >> In case setting like below,
> >>  - ip link set can0 type can bitrate 800000
> >>  - clock=50MHz
> >>  - Use pch_can
> >>
> >> Can-core calculates like below
> >> brp=21
> >> seg1=1
> >> seg2=1
> >> sjw=1
> >> prop_seg=0
> >>
> >> Is "prop_seg=0" true ?
> > 
> > Well, only prop_seg+phase_seg=tseg1 is relevant and the 
> pch_can driver 
> > sets the allowed minimum "tseg1_min1" currently to 1:
> > 
> > static struct can_bittiming_const pch_can_bittiming_const = {
> >         .name = KBUILD_MODNAME,
> >         .tseg1_min = 1,
> >         .tseg1_max = 16,
> >         .tseg2_min = 1,
> >         .tseg2_max = 8,
> >         .sjw_max = 4,
> >         .brp_min = 1,
> >         .brp_max = 1024, /* 6bit + extended 4bit */
> >         .brp_inc = 1,
> > };
> > 
> >> seg1/seg2/sjw/prop_seg must be more than 1 ?
> > 
> > Then "tseg1_min" should be set to *2*.
> > 
> >> Also I can see the following kernel error log.
> >> bitrate error 0.7%
> > 
> > A clock frequency of 50 MHz is sub-optimal for CAN and some 
> bit-rates 
> > cannot be reproduced properly. Here is the output of the can-utils 
> > program "can-calc-bit-timing" (with an entry for the pch-can added):
> > 
> > $ ./can-calc-bit-timing pch-can
> > Bit timing parameters for pch-can with 50.000000 MHz ref clock
> > nominal                                 real Bitrt   nom  real SampP
> > Bitrate TQ[ns] PrS PhS1 PhS2 SJW BRP Bitrate Error SampP 
> SampP Error CNF1 CNF2 CNF3
> > 1000000    100   3    3    3   1   5 1000000  0.0% 75.0% 
> 70.0%  6.7% 0x05 0x92 0x02
> >  800000    420   0    1    1   1  21  793650  0.8% 80.0% 
> 66.6% 16.8% 0x15 0xff 0x00
> >  500000    100   8    8    3   1   5  500000  0.0% 87.5% 
> 85.0%  2.9% 0x05 0xbf 0x02
> >  250000    500   3    3    1   1  25  250000  0.0% 87.5% 
> 87.5%  0.0% 0x19 0x92 0x00
> >  125000    500   6    7    2   1  25  125000  0.0% 87.5% 
> 87.5%  0.0% 0x19 0xb5 0x01
> >  100000    500   8    8    3   1  25  100000  0.0% 87.5% 
> 85.0%  2.9% 0x19 0xbf 0x02
> >   50000   2500   3    3    1   1 125   50000  0.0% 87.5% 
> 87.5%  0.0% 0x7d 0x92 0x00
> >   20000   2500   8    8    3   1 125   20000  0.0% 87.5% 
> 85.0%  2.9% 0x7d 0xbf 0x02
> >   10000  12500   3    3    1   1 625   10000  0.0% 87.5% 
> 87.5%  0.0% 0x71 0x92 0x00
> > 
> > As you can see, especially 800000 gives rather bad results.
> 
> BTW, it's always possible to specify optimized bit-timing 
> parameters directly, e.g. the following seem better:
> 
>    800000     60  12    4    4   4   3  793650  0.8% 80.0% 81.0%  1.2%
> 
> You could set these with:
> 
>   $ ip link set can0 type can \
>     tq 60 prop-seg 12 phase-seg1 4 phase-seg2 4 sjw 4
> 
> Wolfgang.
> 

^ permalink raw reply	[flat|nested] 15+ messages in thread

* RE: About bittiming calculation result
       [not found]                     ` <E2BAACFF191C4175854E6B2EB9135BE5-c0cKtqp5df7I9507bXv2FdBPR1lH4CV8@public.gmane.org>
@ 2011-02-08  3:29                       ` Bhupesh SHARMA
       [not found]                         ` <D5ECB3C7A6F99444980976A8C6D896384DEE2BE1A7-8vAmw3ZAcdzhJTuQ9jeba9BPR1lH4CV8@public.gmane.org>
  0 siblings, 1 reply; 15+ messages in thread
From: Bhupesh SHARMA @ 2011-02-08  3:29 UTC (permalink / raw)
  To: Tomoya MORINAGA, 'Wolfgang Grandegger'
  Cc: socketcan-core-0fE9KPoRgkgATYTw5x5z8w@public.gmane.org,
	netdev-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org

Hi Tomoya,

> -----Original Message-----
> From: socketcan-core-bounces-0fE9KPoRgkgATYTw5x5z8w@public.gmane.org [mailto:socketcan-core-
> bounces-0fE9KPoRgkgATYTw5x5z8w@public.gmane.org] On Behalf Of Tomoya MORINAGA
> Sent: Tuesday, February 08, 2011 6:40 AM
> To: 'Wolfgang Grandegger'
> Cc: socketcan-core-0fE9KPoRgkgATYTw5x5z8w@public.gmane.org; netdev-u79uwXL29TY76Z2rM5mHXA@public.gmane.org; linux-
> kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
> Subject: RE: About bittiming calculation result
> 
> On Monday, February 07, 2011 9:01 PM, Wolfgang Grandegger wrote
> >
> > Well, only prop_seg+phase_seg=tseg1 is relevant and the
> > pch_can driver sets the allowed minimum "tseg1_min1" currently to 1:
> >
> > static struct can_bittiming_const pch_can_bittiming_const = {
> >         .name = KBUILD_MODNAME,
> >         .tseg1_min = 1,
> >         .tseg1_max = 16,
> >         .tseg2_min = 1,
> >         .tseg2_max = 8,
> >         .sjw_max = 4,
> >         .brp_min = 1,
> >         .brp_max = 1024, /* 6bit + extended 4bit */
> >         .brp_inc = 1,
> > };
> >
> > > seg1/seg2/sjw/prop_seg must be more than 1 ?
> >
> > Then "tseg1_min" should be set to *2*.
> 
> Though some drivers accepted by upstream have parameter "tseg1_min" as
> 1,
> Sould we release the patch like below ?
>  -         .tseg1_min = 1,
> +         .tseg1_min = 2,
> 
AFAIK pch uses the Bosch C_CAN core internally.
As per Bosch C_CAN user manual tseg1= prop_seg + phase_seg1
So, ideally tseg1_min should be 2. My version of Bosch C_CAN driver
Uses the value 2.

Regards,
Bhupesh

^ permalink raw reply	[flat|nested] 15+ messages in thread

* RE: About bittiming calculation result
       [not found]                         ` <D5ECB3C7A6F99444980976A8C6D896384DEE2BE1A7-8vAmw3ZAcdzhJTuQ9jeba9BPR1lH4CV8@public.gmane.org>
@ 2011-02-08  4:11                           ` Tomoya MORINAGA
  0 siblings, 0 replies; 15+ messages in thread
From: Tomoya MORINAGA @ 2011-02-08  4:11 UTC (permalink / raw)
  To: 'Bhupesh SHARMA', 'Wolfgang Grandegger'
  Cc: socketcan-core-0fE9KPoRgkgATYTw5x5z8w,
	netdev-u79uwXL29TY76Z2rM5mHXA,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA

Hi  Bhupesh,

On Tuesday, February 08, 2011 12:30 PM, Bhupesh SHARMA wrote:
> AFAIK pch uses the Bosch C_CAN core internally.
> As per Bosch C_CAN user manual tseg1= prop_seg + phase_seg1 
> So, ideally tseg1_min should be 2. My version of Bosch C_CAN 
> driver Uses the value 2.

Thank you for your suggestions.
I will submit the patch.

With Best Regards,
-----------------------------------------
Tomoya MORINAGA
OKI SEMICONDUCTOR CO., LTD.

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: About bittiming calculation result
       [not found]                         ` <93C12206407640199DCDD3A89A333F13-c0cKtqp5df7I9507bXv2FdBPR1lH4CV8@public.gmane.org>
@ 2011-02-08  7:57                           ` Wolfgang Grandegger
  0 siblings, 0 replies; 15+ messages in thread
From: Wolfgang Grandegger @ 2011-02-08  7:57 UTC (permalink / raw)
  To: Tomoya MORINAGA
  Cc: socketcan-core-0fE9KPoRgkgATYTw5x5z8w,
	netdev-u79uwXL29TY76Z2rM5mHXA,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA

Hi Tomoya,

On 02/08/2011 02:27 AM, Tomoya MORINAGA wrote:
> On Tuesday, February 08, 2011 12:53 AM,  Wolfgang Grandegger wrote:
> 
>> BTW, it's always possible to specify optimized bit-timing 
>> parameters directly, e.g. the following seem better:
>>
>>    800000     60  12    4    4   4   3  793650  0.8% 80.0% 81.0%  1.2%
>>
>> You could set these with:
>>
>>   $ ip link set can0 type can \
>>     tq 60 prop-seg 12 phase-seg1 4 phase-seg2 4 sjw 4
> 
> I can confirm 800K comms works well using the above.

Cool, I got these magic values from a CAN hardware expert.

> I wish Can-core could calculate like above.

Me too! I also got some indication on how to improve our algorithm in
case the bit-rate does not match. Hope to find some time soon to work
on this issue.

>>> seg1/seg2/sjw/prop_seg must be more than 1 ?
> BTW, according to EG20T PCH data sheet, 
> CAN bit-timing parameters(BRP, Prop_Seg, Phase_Seg1, Phase_Seg2, SJW) must not be set 0.

OK, then please provide a patch setting tseg1_min to 2.

Wolfgang.

^ permalink raw reply	[flat|nested] 15+ messages in thread

end of thread, other threads:[~2011-02-08  7:57 UTC | newest]

Thread overview: 15+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2011-02-04 12:22 [PATCH 00/20] world-writable files in sysfs and debugfs Vasiliy Kulikov
2011-02-04 12:23 ` [PATCH 12/20] net: can: at91_can: world-writable sysfs files Vasiliy Kulikov
     [not found]   ` <a6800dc8b0daed78256f98f52844cbbb48f4a76d.1296818921.git.segoon-cxoSlKxDwOJWk0Htik3J/w@public.gmane.org>
2011-02-04 12:42     ` Kurt Van Dijck
     [not found]       ` <20110204124233.GB334-MxZ6Iy/zr/UdbCeoMzGj59i2O/JbrIOy@public.gmane.org>
2011-02-04 21:06         ` David Miller
2011-02-07 11:38           ` About bittiming calculation result Tomoya MORINAGA
     [not found]             ` <5009516791F146C49C73FAC57C437313-c0cKtqp5df7I9507bXv2FdBPR1lH4CV8@public.gmane.org>
2011-02-07 12:00               ` Wolfgang Grandegger
     [not found]                 ` <4D4FDEF9.2030305-5Yr1BZd7O62+XT7JhA+gdA@public.gmane.org>
2011-02-07 15:52                   ` Wolfgang Grandegger
     [not found]                     ` <4D501555.5000905-5Yr1BZd7O62+XT7JhA+gdA@public.gmane.org>
2011-02-08  1:27                       ` Tomoya MORINAGA
     [not found]                         ` <93C12206407640199DCDD3A89A333F13-c0cKtqp5df7I9507bXv2FdBPR1lH4CV8@public.gmane.org>
2011-02-08  7:57                           ` Wolfgang Grandegger
2011-02-08  1:09                   ` Tomoya MORINAGA
     [not found]                     ` <E2BAACFF191C4175854E6B2EB9135BE5-c0cKtqp5df7I9507bXv2FdBPR1lH4CV8@public.gmane.org>
2011-02-08  3:29                       ` Bhupesh SHARMA
     [not found]                         ` <D5ECB3C7A6F99444980976A8C6D896384DEE2BE1A7-8vAmw3ZAcdzhJTuQ9jeba9BPR1lH4CV8@public.gmane.org>
2011-02-08  4:11                           ` Tomoya MORINAGA
2011-02-04 12:23 ` [PATCH 13/20] net: can: janz-ican3: world-writable sysfs termination file Vasiliy Kulikov
     [not found]   ` <6b49b9521416fbd50214485d3e14e5f254ada4f7.1296818921.git.segoon-cxoSlKxDwOJWk0Htik3J/w@public.gmane.org>
2011-02-04 21:06     ` David Miller
2011-02-04 13:11 ` [rtc-linux] [PATCH 00/20] world-writable files in sysfs and debugfs Linus Walleij

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).