Don't allow everybody to write to mb0_id file.
Signed-off-by: Vasiliy Kulikov <[email protected]>
---
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
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 <[email protected]>
From: Kurt Van Dijck <[email protected]>
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 <[email protected]>
Applied.
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.
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.
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.
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.
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:[email protected]]
> Sent: Tuesday, February 08, 2011 12:53 AM
> To: Tomoya MORINAGA
> Cc: [email protected]; [email protected];
> [email protected]
> 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.
>
Hi Tomoya,
> -----Original Message-----
> From: [email protected] [mailto:socketcan-core-
> [email protected]] On Behalf Of Tomoya MORINAGA
> Sent: Tuesday, February 08, 2011 6:40 AM
> To: 'Wolfgang Grandegger'
> Cc: [email protected]; [email protected]; linux-
> [email protected]
> 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
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.
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.