Re: [PATCH] parisc: math-emu: Few spelling fixes in the file fpu.h

[Jeroen Roovers]

On Wed, 17 Mar 2021 16:02:51 +0530
Bhaskar Chowdhury  wrote:

> s/synopis/synopsis/
> s/differeniate/differentiate/
> s/differeniation/differentiation/
> 
> Signed-off-by: Bhaskar Chowdhury 
> ---
>  arch/parisc/math-emu/fpu.h | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/arch/parisc/math-emu/fpu.h b/arch/parisc/math-emu/fpu.h
> index 853c19c03828..1f313bc38beb 100644
> --- a/arch/parisc/math-emu/fpu.h
> +++ b/arch/parisc/math-emu/fpu.h
> @@ -12,7 +12,7 @@
>   *  @(#) pa/fp/fpu.h $Revision: 1.1 $
>   *
>   *  Purpose:
> - *  < by this file>>
> + *  < provided by this file>> *
>   *
>   * END_DESC
> @@ -50,9 +50,9 @@
>  #define EMULATION_VERSION 4
> 
>  /*
> - * The only was to differeniate between TIMEX and ROLEX (or PCX-S
> and PCX-T)
> + * The only was to differentiate between TIMEX and ROLEX (or PCX-S
> and PCX-T)

Might as well fix "only [change] was to" here.

>   * is thorough the potential type field from the PDC_MODEL call.  The
> - * following flags are used at assist this differeniation.
> + * following flags are used at assist this differentiation.
>   */
> 
>  #define ROLEX_POTENTIAL_KEY_FLAGSPDC_MODEL_CPU_KEY_WORD_TO_IO
> --
> 2.30.2
> 


Kind regards,
 jer

Re: [PATCH 1/1] can: dev: add software tx timestamps

[Jeroen Hofstee]

Hello Vincent,

On 1/10/21 11:35 AM, Vincent Mailhol wrote:

Call skb_tx_timestamp() within can_put_echo_skb() so that a software
tx timestamp gets attached on the skb.


[..]


diff --git a/drivers/net/can/dev.c b/drivers/net/can/dev.c
index 3486704c8a95..3904e0874543 100644
--- a/drivers/net/can/dev.c
+++ b/drivers/net/can/dev.c
@@ -484,6 +484,8 @@ int can_put_echo_skb(struct sk_buff *skb, struct net_device 
*dev,
  
  		/* save this skb for tx interrupt echo handling */

priv->echo_skb[idx] = skb;
+
+   skb_tx_timestamp(skb);
} else {
/* locking problem with netif_stop_queue() ?? */
netdev_err(dev, "%s: BUG! echo_skb %d is occupied!\n", 
__func__, idx);


Personally, I would put the skb_tx_timestamp, before adding it to the array:

    /* make settings for echo to reduce code in irq context */
    skb->pkt_type = PACKET_BROADCAST;
    skb->ip_summed = CHECKSUM_UNNECESSARY;
    skb->dev = dev;
+   skb_tx_timestamp(skb);

    /* save this skb for tx interrupt echo handling */
    priv->echo_skb[idx] = skb;


I don't think it actually matters though.

Regards,

Jeroen

[PATCH] can: don't count arbitration lose as an error

[Jeroen Hofstee]

Losing arbitration is normal in a CAN-bus network, it means that a
higher priority frame is being send and the pending message will be
retried later. Hence most driver only increment arbitration_lost, but
the sja1000 and sun4i driver also incremeant tx_error, causing errors
to be reported on a normal functioning CAN-bus. So stop counting them
as errors.

For completeness, the Kvaser USB hybra also increments the tx_error
on arbitration lose, but it does so in single shot. Since in that
case the message is not retried, that behaviour is kept.

Signed-off-by: Jeroen Hofstee 
---
 drivers/net/can/sja1000/sja1000.c | 1 -
 drivers/net/can/sun4i_can.c   | 1 -
 2 files changed, 2 deletions(-)

diff --git a/drivers/net/can/sja1000/sja1000.c 
b/drivers/net/can/sja1000/sja1000.c
index 9f107798f904..25a4d7d0b349 100644
--- a/drivers/net/can/sja1000/sja1000.c
+++ b/drivers/net/can/sja1000/sja1000.c
@@ -474,7 +474,6 @@ static int sja1000_err(struct net_device *dev, uint8_t 
isrc, uint8_t status)
netdev_dbg(dev, "arbitration lost interrupt\n");
alc = priv->read_reg(priv, SJA1000_ALC);
priv->can.can_stats.arbitration_lost++;
-   stats->tx_errors++;
cf->can_id |= CAN_ERR_LOSTARB;
cf->data[0] = alc & 0x1f;
}
diff --git a/drivers/net/can/sun4i_can.c b/drivers/net/can/sun4i_can.c
index e2c6cf4b2228..b3f2f4fe5ee0 100644
--- a/drivers/net/can/sun4i_can.c
+++ b/drivers/net/can/sun4i_can.c
@@ -604,7 +604,6 @@ static int sun4i_can_err(struct net_device *dev, u8 isrc, 
u8 status)
netdev_dbg(dev, "arbitration lost interrupt\n");
alc = readl(priv->base + SUN4I_REG_STA_ADDR);
priv->can.can_stats.arbitration_lost++;
-   stats->tx_errors++;
if (likely(skb)) {
cf->can_id |= CAN_ERR_LOSTARB;
cf->data[0] = (alc >> 8) & 0x1f;
-- 
2.17.1

Re: [PATCH] can: m_can: fix boolreturn.cocci warnings

[Jeroen Hofstee]

Hello Simon,

On 10/15/19 9:13 AM, Simon Horman wrote:
> On Tue, Oct 15, 2019 at 06:37:54AM +0000, Jeroen Hofstee wrote:
>> Hi,
>>
>> On 10/15/19 7:57 AM, Simon Horman wrote:
>>> On Mon, Oct 14, 2019 at 11:04:28PM +0800, kbuild test robot wrote:
>>>> From: kbuild test robot 
>>>>
>>>> drivers/net/can/m_can/m_can.c:783:9-10: WARNING: return of 0/1 in function 
>>>> 'is_protocol_err' with return type bool
>>>>
>>>>Return statements in functions returning bool should use
>>>>true/false instead of 1/0.
>>>> Generated by: scripts/coccinelle/misc/boolreturn.cocci
>>>>
>>>> Fixes: 46946163ac61 ("can: m_can: add support for handling arbitration 
>>>> error")
>>>> CC: Pankaj Sharma 
>>>> Signed-off-by: kbuild test robot 
>>>> ---
>>>>
>>>> url:
>>>> https://github.com/0day-ci/linux/commits/Pankaj-Sharma/can-m_can-add-support-for-handling-arbitration-error/20191014-193532
>>>>
>>>>m_can.c |4 ++--
>>>>1 file changed, 2 insertions(+), 2 deletions(-)
>>>>
>>>> --- a/drivers/net/can/m_can/m_can.c
>>>> +++ b/drivers/net/can/m_can/m_can.c
>>>> @@ -780,9 +780,9 @@ static inline bool is_lec_err(u32 psr)
>>>>static inline bool is_protocol_err(u32 irqstatus)
>>>>{
>>>>if (irqstatus & IR_ERR_LEC_31X)
>>>> -  return 1;
>>>> +  return true;
>>>>else
>>>> -  return 0;
>>>> +  return false;
>>>>}
>>>>
>>>>static int m_can_handle_protocol_error(struct net_device *dev, u32 
>>>> irqstatus)
>>>>
>>> <2c>
>>> Perhaps the following is a nicer way to express this (completely untested):
>>>
>>> return !!(irqstatus & IR_ERR_LEC_31X);
>>> 
>>
>> Really, !! for bool / _Bool types? why not simply:
>>
>> static inline bool is_protocol_err(u32 irqstatus)
>>  return irqstatus & IR_ERR_LEC_31X;
>> }
> Good point, silly me.


For clarity, I am commenting on the suggestion made by
the tool, not the patch itself..

Regards,

Jeroen

Re: [PATCH] can: m_can: fix boolreturn.cocci warnings

[Jeroen Hofstee]

Hi,

On 10/15/19 7:57 AM, Simon Horman wrote:
> On Mon, Oct 14, 2019 at 11:04:28PM +0800, kbuild test robot wrote:
>> From: kbuild test robot 
>>
>> drivers/net/can/m_can/m_can.c:783:9-10: WARNING: return of 0/1 in function 
>> 'is_protocol_err' with return type bool
>>
>>   Return statements in functions returning bool should use
>>   true/false instead of 1/0.
>> Generated by: scripts/coccinelle/misc/boolreturn.cocci
>>
>> Fixes: 46946163ac61 ("can: m_can: add support for handling arbitration 
>> error")
>> CC: Pankaj Sharma 
>> Signed-off-by: kbuild test robot 
>> ---
>>
>> url:
>> https://github.com/0day-ci/linux/commits/Pankaj-Sharma/can-m_can-add-support-for-handling-arbitration-error/20191014-193532
>>
>>   m_can.c |4 ++--
>>   1 file changed, 2 insertions(+), 2 deletions(-)
>>
>> --- a/drivers/net/can/m_can/m_can.c
>> +++ b/drivers/net/can/m_can/m_can.c
>> @@ -780,9 +780,9 @@ static inline bool is_lec_err(u32 psr)
>>   static inline bool is_protocol_err(u32 irqstatus)
>>   {
>>  if (irqstatus & IR_ERR_LEC_31X)
>> -return 1;
>> +return true;
>>  else
>> -return 0;
>> +return false;
>>   }
>>   
>>   static int m_can_handle_protocol_error(struct net_device *dev, u32 
>> irqstatus)
>>
> <2c>
> Perhaps the following is a nicer way to express this (completely untested):
>
>   return !!(irqstatus & IR_ERR_LEC_31X);
> 


Really, !! for bool / _Bool types? why not simply:

static inline bool is_protocol_err(u32 irqstatus)
return irqstatus & IR_ERR_LEC_31X;
}

Regards,
Jeroen

Re: [PATCH 1/7] can: rx-offload: continue on error

[Jeroen Hofstee]

Hello Marc,

On 10/9/19 3:18 PM, Marc Kleine-Budde wrote:
> Hello Jeroen,
>
> I'm currently looking at the rx-offload error handling in detail.
>
> TLDR: I've added the patch to linux-can.
>
> Thanks,
> Marc
>
> For the record, the details:
>
> On 9/24/19 8:45 PM, Jeroen Hofstee wrote:
>> While can_rx_offload_offload_one will call mailbox_read to discard
>> the mailbox in case of an error,
> Yes.
>
> can_rx_offload_offload_one() will read into the discard mailbox in case
> of an error.
>
> Currently there are two kinds of errors:
> 1) the rx-offoad skb queue (between the IRQ handler and the NAPI)
> is full
> 2) alloc_can_skb() fails to allocate a skb, due to OOM
>
>> can_rx_offload_irq_offload_timestamp bails out in the error case.
> Yes, in case of an error both can_rx_offload_irq_offload_timestamp() and
> can_rx_offload_irq_offload_fifo() will stop reading mailboxes, add the
> already filled skbs to the queue and schedule NAPI if needed.
>
> Currently both can_rx_offload_irq_offload_timestamp() and
> can_rx_offload_irq_offload_fifo() will return the number of queued
> mailboxes.
>
> This means in case of queue overflow or OOM, only one mailbox is
> discaded, before can_rx_offload_irq_offload_*() returning the number of
> successfully queued mailboxes so far.
>
> Looking at the flexcan driver:
>
> https://elixir.bootlin.com/linux/latest/source/drivers/net/can/flexcan.c#L867
>
>>  while ((reg_iflag = flexcan_read_reg_iflag_rx(priv))) {
>>  handled = IRQ_HANDLED;
>>  ret = 
>> can_rx_offload_irq_offload_timestamp(&priv->offload,
>> reg_iflag);
>>  if (!ret)
>>  break;
>>  }
> [...]
>>  if (reg_iflag1 & FLEXCAN_IFLAG_RX_FIFO_AVAILABLE) {
>>  handled = IRQ_HANDLED;
>>  can_rx_offload_irq_offload_fifo(&priv->offload);
>>  }
> This means for the timestamp mode, at least one mailbox is discarded or
> if the error occurred after reading one or more mailboxes the while loop
> will try again. If the error persists a second mailbox is discarded.
>
> For the fifo mode, only one mailbox is discarded.
>
> Then the flexcan's IRQ is exited. If there are messages in mailboxes are
> pending another IRQ is triggered I doubt that this is a good idea.
>
> On the other hand the ti_hecc driver:
>
>>  /* offload RX mailboxes and let NAPI deliver them */
>>  while ((rx_pending = hecc_read(priv, HECC_CANRMP))) {
>>  can_rx_offload_irq_offload_timestamp(&priv->offload,
>>   rx_pending);
>>  hecc_write(priv, HECC_CANRMP, rx_pending);
>>  }
> The error is ignored and the _all_ mailboxes are discarded (given the
> error persists).
>
>> Since it is typically called from a while loop, all message will
>> eventually be discarded. So lets continue on error instead to discard
>> them directly.
> After reading my own code and writing it up, your patch totally makes sense.
>
> If there is a shortage of resources, queue overrun or OOM, it makes no
> sense to return from the IRQ handler, if a mailbox is still active as it
> will trigger the IRQ again. Entering the IRQ handler again probably
> doesn't give the system time to recover from the resource problem.


Indeed, I have nothing to comment on that, besides thanks for
being willing to reconsider your own code.

With kind regards,

Jeroen

Re: [PATCH] ARM: dts: am335x-sancloud-bbe: Fix PHY mode for ethernet

[Jeroen Hofstee]

Hello Grygorri,

On 10/9/19 11:43 AM, Grygorii Strashko wrote:
>
>
>>> Grygorii doesn't suggest to add a fixes tag, just to change the 
>>> referenced
>>> commit to another. Obviously I would like to understand why another 
>>> commit
>>> should be referenced. And then I should read and parse the response, 
>>> so there
>>> is no special reason, just time...
>>
>> OK sure. Well once you guys have the commit figured out, let me
>> know what to apply. And we know Grygorii is mostly right based
>> on his history of comments so best to not ignore that :)
>
> Sry, but I do not think my request is somehow special.
> Yes, your patch is correct by itself, but commit description is not:
> 1) commit cd28d1d6e52e: ("net: phy: at803x: Disable phy delay for 
> RGMII mode")  which you've mentioned is A BUG
> and should not be merged first of all (which you can find out by 
> reading corresponding thread).
>
> just try checkout that commit and apply your patch on top - networking 
> should not work.
>
> But it was merged and not reverted - instead two more patches were 
> applied to fix regression.
>
> 2) Those commits are defined final behavior (which i again explained 
> above) and that new behavior hardly can
> be called "the bug in the at803x driver" as, unfortunately, there were 
> no common conclusion how default values for
> RX/TX delay should be handled vs phy-mode = "rgmii-txid"/"rgmii-rxid".
> Originally many PHY driver kept them default (as per boot 
> strapping/bootloader configuration), but now
> some driver (including at803x) started disabling RX delay if 
> "rgmii-txid" or TX delay if "rgmii-rxid".
>
> Hence, pls update commit message and add proper fixes tag. smth like:
> "Now after commit 6d4cd041f0af net: phy: at803x: disable delay only 
> for RGMII mode the driver will forcibly disable
> RGMII RX delay if phy-mode = "rgmii-txid" is specified in DT which 
> will break networking on ..
>
> Hence change .. to ensure ...
> Fixes: "
>
>

Yes, that part is clear to me, and I am not saying you are incorrect,
only that I would like to understand why above commit pops up
(since it makes no sense to me as well, my own commit). And the reason
is rather silly, I guess... I fixed it on master, thereafter checked the 
5.1 branch,
while keeping the fixed dtb... :(

Let me check that and I will send a v2. Likely tomorrow
(I am not near the device at the moment).

Regards,

Jeroen

Re: [PATCH] ARM: dts: am335x-sancloud-bbe: Fix PHY mode for ethernet

[Jeroen Hofstee]

Hi,

On 10/8/19 6:51 PM, Tony Lindgren wrote:
> * Jeroen Hofstee  [191008 16:43]:
>> Hello Tony,
>>
>> On 10/8/19 6:14 PM, Tony Lindgren wrote:
>>> * Jeroen Hofstee  [191008 16:03]:
>>>> Hello Tony,
>>>>
>>>> On 10/8/19 4:23 PM, Tony Lindgren wrote:
>>>>> * Grygorii Strashko  [191003 02:32]:
>>>>>> On 03/10/2019 11:16, Jeroen Hofstee wrote:
>>>>>>> Furthermore 4.19 is fine, so there is no need to include it in stable
>>>>>>> and have a note to make sure also other patches are required etc.
>>>>>> Hence all above patches went in 5.1 it would be correct to mention only
>>>>>> 6d4cd041f0af net: phy: at803x: disable delay only for RGMII mode
>>>>> Jeroen, can you please send an updated patch with the fixes
>>>>> tag changed?
>>>>>
>>>> For completeness, there is no "Fixes tag" as you mentioned.
>>>> The commit only refers to another commit which introduces
>>>> a problem.
>>> Well please add the fixes tag, that way this will get
>>> properly applied to earlier stable kernels too :)
>> But 4.19 is fine, this is an issue in 5.1 as in EOL...
>> I really don't understand why I should waste time
>> to figure out what happened exactly during the 5.1
>> release cycle...
> Hmm so what's the issue with just adding the fixes tag Grygorii
> suggested:
>
> 6d4cd041f0af ("net: phy: at803x: disable delay only for RGMII mode")
>
> No need to dig further?

Grygorii doesn't suggest to add a fixes tag, just to change the referenced
commit to another. Obviously I would like to understand why another commit
should be referenced. And then I should read and parse the response, so there
is no special reason, just time...

Regards,
Jeroen

Re: [PATCH] ARM: dts: am335x-sancloud-bbe: Fix PHY mode for ethernet

[Jeroen Hofstee]

Hello Tony,

On 10/8/19 6:14 PM, Tony Lindgren wrote:
> * Jeroen Hofstee  [191008 16:03]:
>> Hello Tony,
>>
>> On 10/8/19 4:23 PM, Tony Lindgren wrote:
>>> * Grygorii Strashko  [191003 02:32]:
>>>> On 03/10/2019 11:16, Jeroen Hofstee wrote:
>>>>> Furthermore 4.19 is fine, so there is no need to include it in stable
>>>>> and have a note to make sure also other patches are required etc.
>>>> Hence all above patches went in 5.1 it would be correct to mention only
>>>> 6d4cd041f0af net: phy: at803x: disable delay only for RGMII mode
>>> Jeroen, can you please send an updated patch with the fixes
>>> tag changed?
>>>
>> For completeness, there is no "Fixes tag" as you mentioned.
>> The commit only refers to another commit which introduces
>> a problem.
> Well please add the fixes tag, that way this will get
> properly applied to earlier stable kernels too :)

But 4.19 is fine, this is an issue in 5.1 as in EOL...
I really don't understand why I should waste time
to figure out what happened exactly during the 5.1
release cycle...

Regards,

Jeroen

Re: [PATCH] ARM: dts: am335x-sancloud-bbe: Fix PHY mode for ethernet

[Jeroen Hofstee]

Hello Tony,

On 10/8/19 4:23 PM, Tony Lindgren wrote:
> * Grygorii Strashko  [191003 02:32]:
>> On 03/10/2019 11:16, Jeroen Hofstee wrote:
>>> Furthermore 4.19 is fine, so there is no need to include it in stable
>>> and have a note to make sure also other patches are required etc.
>> Hence all above patches went in 5.1 it would be correct to mention only
>> 6d4cd041f0af net: phy: at803x: disable delay only for RGMII mode
> Jeroen, can you please send an updated patch with the fixes
> tag changed?
>

For completeness, there is no "Fixes tag" as you mentioned.
The commit only refers to another commit which introduces
a problem.

Regards,

Jeroen

Re: [PATCH] ARM: dts: am335x-sancloud-bbe: Fix PHY mode for ethernet

[Jeroen Hofstee]

Hello Tony,

On 10/8/19 4:23 PM, Tony Lindgren wrote:
> * Grygorii Strashko  [191003 02:32]:
>> On 03/10/2019 11:16, Jeroen Hofstee wrote:
>>> Furthermore 4.19 is fine, so there is no need to include it in stable
>>> and have a note to make sure also other patches are required etc.
>> Hence all above patches went in 5.1 it would be correct to mention only
>> 6d4cd041f0af net: phy: at803x: disable delay only for RGMII mode
> Jeroen, can you please send an updated patch with the fixes
> tag changed?


Not at the moment. I am right that the mentioned commit
is the first one to break the ethernet. Grygorii is right it
seems that that commit shouldn't affect it, yet it does.

So I would like to understand how it breaks things so I can
up with a sensible commit message (or we just drop reference
to other commits so I don't have to dig through the 5.1 history,
the patch by itself is also valid).

Regards,

Jeroen

Re: [PATCH] ARM: dts: am335x-sancloud-bbe: Fix PHY mode for ethernet

[Jeroen Hofstee]

Hello Grygorri,

On 10/2/19 4:48 PM, Grygorii Strashko wrote:
>
>
> On 02/10/2019 12:54, Jeroen Hofstee wrote:
>> cd28d1d6e52e: ("net: phy: at803x: Disable phy delay for RGMII mode") 
>> broke
>> the ethernet networking on the beaglebone enhanced.
>
> Above commit is incorrect (by itself) and there are few more commits 
> on top of
> it, so pls. update reference to commit(s)
>
> bb0ce4c1517d net: phy: at803x: stop switching phy delay config needlessly
> 6d4cd041f0af net: phy: at803x: disable delay only for RGMII mode
>
>
I don't see why that is relevant. The mention patch introduces a
backwards incompatibility for the device tree. The patches you
mention don't fix that and hence are unrelated to this patch.

Furthermore 4.19 is fine, so there is no need to include it in stable
and have a note to make sure also other patches are required etc.

Regards,

Jeroen

[PATCH] ARM: dts: am335x-sancloud-bbe: Fix PHY mode for ethernet

[Jeroen Hofstee]

cd28d1d6e52e: ("net: phy: at803x: Disable phy delay for RGMII mode") broke
the ethernet networking on the beaglebone enhanced.

The board relied on the bug in the at803x driver to always enable the rx
delay. So change the phy-mode to rgmii-id so it is enabled again.

Signed-off-by: Jeroen Hofstee 
cc: Koen Kooi 
---
 arch/arm/boot/dts/am335x-sancloud-bbe.dts | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/arm/boot/dts/am335x-sancloud-bbe.dts 
b/arch/arm/boot/dts/am335x-sancloud-bbe.dts
index 8678e6e35493..e5fdb7abb0d5 100644
--- a/arch/arm/boot/dts/am335x-sancloud-bbe.dts
+++ b/arch/arm/boot/dts/am335x-sancloud-bbe.dts
@@ -108,7 +108,7 @@
 
 &cpsw_emac0 {
phy-handle = <ðphy0>;
-   phy-mode = "rgmii-txid";
+   phy-mode = "rgmii-id";
 };
 
 &i2c0 {
-- 
2.17.1

Re: [PATCH 1/2] can: D_CAN: perform a sofware reset on open

[Jeroen Hofstee]

Hello Marc,

On 10/1/19 4:32 PM, Marc Kleine-Budde wrote:
> On 9/26/19 10:50 AM, Jeroen Hofstee wrote:
>> When the C_CAN interface is closed it is put in power down mode, but
>> does not reset the error counters / state. So reset the D_CAN on open,
>> so the reported state and the actual state match.
>>
>> According to [1], the C_CAN module doesn't have the software reset.
>>
>> [1] 
>> http://www.bosch-semiconductors.com/media/ip_modules/pdf_2/c_can_fd8/users_manual_c_can_fd8_r210_1.pdf
>>
>> Signed-off-by: Jeroen Hofstee 
>> ---
>>   drivers/net/can/c_can/c_can.c | 26 ++
>>   1 file changed, 26 insertions(+)
>>
>> diff --git a/drivers/net/can/c_can/c_can.c b/drivers/net/can/c_can/c_can.c
>> index 606b7d8ffe13..502a181d02e7 100644
>> --- a/drivers/net/can/c_can/c_can.c
>> +++ b/drivers/net/can/c_can/c_can.c
>> @@ -52,6 +52,7 @@
>>   #define CONTROL_EX_PDR BIT(8)
>>   
>>   /* control register */
>> +#define CONTROL_SWR BIT(15)
>>   #define CONTROL_TEST   BIT(7)
>>   #define CONTROL_CCEBIT(6)
>>   #define CONTROL_DISABLE_AR BIT(5)
>> @@ -569,6 +570,26 @@ static void c_can_configure_msg_objects(struct 
>> net_device *dev)
>> IF_MCONT_RCV_EOB);
>>   }
>>   
>> +static int software_reset(struct net_device *dev)
> Please add the common prefix "c_can_" to the function

Fine with me, I did sent a v2.

Regards,
Jeroen

[PATCH v2 1/2] can: D_CAN: perform a sofware reset on open

[Jeroen Hofstee]

When the C_CAN interface is closed it is put in power down mode, but
does not reset the error counters / state. So reset the D_CAN on open,
so the reported state and the actual state match.

According to [1], the C_CAN module doesn't have the software reset.

[1] 
http://www.bosch-semiconductors.com/media/ip_modules/pdf_2/c_can_fd8/users_manual_c_can_fd8_r210_1.pdf

Signed-off-by: Jeroen Hofstee 
---
 drivers/net/can/c_can/c_can.c | 26 ++
 1 file changed, 26 insertions(+)

diff --git a/drivers/net/can/c_can/c_can.c b/drivers/net/can/c_can/c_can.c
index 606b7d8ffe13..2332687fa6dc 100644
--- a/drivers/net/can/c_can/c_can.c
+++ b/drivers/net/can/c_can/c_can.c
@@ -52,6 +52,7 @@
 #define CONTROL_EX_PDR BIT(8)
 
 /* control register */
+#define CONTROL_SWRBIT(15)
 #define CONTROL_TEST   BIT(7)
 #define CONTROL_CCEBIT(6)
 #define CONTROL_DISABLE_AR BIT(5)
@@ -569,6 +570,26 @@ static void c_can_configure_msg_objects(struct net_device 
*dev)
   IF_MCONT_RCV_EOB);
 }
 
+static int c_can_software_reset(struct net_device *dev)
+{
+   struct c_can_priv *priv = netdev_priv(dev);
+   int retry = 0;
+
+   if (priv->type != BOSCH_D_CAN)
+   return 0;
+
+   priv->write_reg(priv, C_CAN_CTRL_REG, CONTROL_SWR | CONTROL_INIT);
+   while (priv->read_reg(priv, C_CAN_CTRL_REG) & CONTROL_SWR) {
+   msleep(20);
+   if (retry++ > 100) {
+   netdev_err(dev, "CCTRL: software reset failed\n");
+   return -EIO;
+   }
+   }
+
+   return 0;
+}
+
 /*
  * Configure C_CAN chip:
  * - enable/disable auto-retransmission
@@ -578,6 +599,11 @@ static void c_can_configure_msg_objects(struct net_device 
*dev)
 static int c_can_chip_config(struct net_device *dev)
 {
struct c_can_priv *priv = netdev_priv(dev);
+   int err;
+
+   err = c_can_software_reset(dev);
+   if (err)
+   return err;
 
/* enable automatic retransmission */
priv->write_reg(priv, C_CAN_CTRL_REG, CONTROL_ENABLE_AR);
-- 
2.17.1

[PATCH v2 2/2] can: C_CAN: add bus recovery events

[Jeroen Hofstee]

While the state is updated when the error counters increase and decrease,
there is no event when the bus recovers and the error counters decrease
again. So add that event as well.

Change the state going downward to be ERROR_PASSIVE -> ERROR_WARNING ->
ERROR_ACTIVE instead of directly to ERROR_ACTIVE again.

Signed-off-by: Jeroen Hofstee 
Acked-by: Kurt Van Dijck 
Tested-by: Kurt Van Dijck 
---
 drivers/net/can/c_can/c_can.c | 20 ++--
 1 file changed, 18 insertions(+), 2 deletions(-)

diff --git a/drivers/net/can/c_can/c_can.c b/drivers/net/can/c_can/c_can.c
index 2332687fa6dc..e1e9b87dd4d2 100644
--- a/drivers/net/can/c_can/c_can.c
+++ b/drivers/net/can/c_can/c_can.c
@@ -912,6 +912,9 @@ static int c_can_handle_state_change(struct net_device *dev,
struct can_berr_counter bec;
 
switch (error_type) {
+   case C_CAN_NO_ERROR:
+   priv->can.state = CAN_STATE_ERROR_ACTIVE;
+   break;
case C_CAN_ERROR_WARNING:
/* error warning state */
priv->can.can_stats.error_warning++;
@@ -942,6 +945,13 @@ static int c_can_handle_state_change(struct net_device 
*dev,
ERR_CNT_RP_SHIFT;
 
switch (error_type) {
+   case C_CAN_NO_ERROR:
+   /* error warning state */
+   cf->can_id |= CAN_ERR_CRTL;
+   cf->data[1] = CAN_ERR_CRTL_ACTIVE;
+   cf->data[6] = bec.txerr;
+   cf->data[7] = bec.rxerr;
+   break;
case C_CAN_ERROR_WARNING:
/* error warning state */
cf->can_id |= CAN_ERR_CRTL;
@@ -1080,11 +1090,17 @@ static int c_can_poll(struct napi_struct *napi, int 
quota)
/* handle bus recovery events */
if ((!(curr & STATUS_BOFF)) && (last & STATUS_BOFF)) {
netdev_dbg(dev, "left bus off state\n");
-   priv->can.state = CAN_STATE_ERROR_ACTIVE;
+   work_done += c_can_handle_state_change(dev, 
C_CAN_ERROR_PASSIVE);
}
+
if ((!(curr & STATUS_EPASS)) && (last & STATUS_EPASS)) {
netdev_dbg(dev, "left error passive state\n");
-   priv->can.state = CAN_STATE_ERROR_ACTIVE;
+   work_done += c_can_handle_state_change(dev, 
C_CAN_ERROR_WARNING);
+   }
+
+   if ((!(curr & STATUS_EWARN)) && (last & STATUS_EWARN)) {
+   netdev_dbg(dev, "left error warning state\n");
+   work_done += c_can_handle_state_change(dev, C_CAN_NO_ERROR);
}
 
/* handle lec errors on the bus */
-- 
2.17.1

[PATCH 1/2] can: D_CAN: perform a sofware reset on open

[Jeroen Hofstee]

When the C_CAN interface is closed it is put in power down mode, but
does not reset the error counters / state. So reset the D_CAN on open,
so the reported state and the actual state match.

According to [1], the C_CAN module doesn't have the software reset.

[1] 
http://www.bosch-semiconductors.com/media/ip_modules/pdf_2/c_can_fd8/users_manual_c_can_fd8_r210_1.pdf

Signed-off-by: Jeroen Hofstee 
---
 drivers/net/can/c_can/c_can.c | 26 ++
 1 file changed, 26 insertions(+)

diff --git a/drivers/net/can/c_can/c_can.c b/drivers/net/can/c_can/c_can.c
index 606b7d8ffe13..502a181d02e7 100644
--- a/drivers/net/can/c_can/c_can.c
+++ b/drivers/net/can/c_can/c_can.c
@@ -52,6 +52,7 @@
 #define CONTROL_EX_PDR BIT(8)
 
 /* control register */
+#define CONTROL_SWRBIT(15)
 #define CONTROL_TEST   BIT(7)
 #define CONTROL_CCEBIT(6)
 #define CONTROL_DISABLE_AR BIT(5)
@@ -569,6 +570,26 @@ static void c_can_configure_msg_objects(struct net_device 
*dev)
   IF_MCONT_RCV_EOB);
 }
 
+static int software_reset(struct net_device *dev)
+{
+   struct c_can_priv *priv = netdev_priv(dev);
+   int retry = 0;
+
+   if (priv->type != BOSCH_D_CAN)
+   return 0;
+
+   priv->write_reg(priv, C_CAN_CTRL_REG, CONTROL_SWR | CONTROL_INIT);
+   while (priv->read_reg(priv, C_CAN_CTRL_REG) & CONTROL_SWR) {
+   msleep(20);
+   if (retry++ > 100) {
+   netdev_err(dev, "CCTRL: software reset failed\n");
+   return -EIO;
+   }
+   }
+
+   return 0;
+}
+
 /*
  * Configure C_CAN chip:
  * - enable/disable auto-retransmission
@@ -578,6 +599,11 @@ static void c_can_configure_msg_objects(struct net_device 
*dev)
 static int c_can_chip_config(struct net_device *dev)
 {
struct c_can_priv *priv = netdev_priv(dev);
+   int err;
+
+   err = software_reset(dev);
+   if (err)
+   return err;
 
/* enable automatic retransmission */
priv->write_reg(priv, C_CAN_CTRL_REG, CONTROL_ENABLE_AR);
-- 
2.17.1

[PATCH 2/2] can: C_CAN: add bus recovery events

[Jeroen Hofstee]

While the state is update when the error counters increase and decrease,
there is no event when the bus recovers and the error counters decrease
again. So add that event as well.

Change the state going downward to be ERROR_PASSIVE -> ERROR_WARNING ->
ERROR_ACTIVE instead of directly to ERROR_ACTIVE again.

Signed-off-by: Jeroen Hofstee 
---
 drivers/net/can/c_can/c_can.c | 20 ++--
 1 file changed, 18 insertions(+), 2 deletions(-)

diff --git a/drivers/net/can/c_can/c_can.c b/drivers/net/can/c_can/c_can.c
index 502a181d02e7..5cfaab18e10b 100644
--- a/drivers/net/can/c_can/c_can.c
+++ b/drivers/net/can/c_can/c_can.c
@@ -912,6 +912,9 @@ static int c_can_handle_state_change(struct net_device *dev,
struct can_berr_counter bec;
 
switch (error_type) {
+   case C_CAN_NO_ERROR:
+   priv->can.state = CAN_STATE_ERROR_ACTIVE;
+   break;
case C_CAN_ERROR_WARNING:
/* error warning state */
priv->can.can_stats.error_warning++;
@@ -942,6 +945,13 @@ static int c_can_handle_state_change(struct net_device 
*dev,
ERR_CNT_RP_SHIFT;
 
switch (error_type) {
+   case C_CAN_NO_ERROR:
+   /* error warning state */
+   cf->can_id |= CAN_ERR_CRTL;
+   cf->data[1] = CAN_ERR_CRTL_ACTIVE;
+   cf->data[6] = bec.txerr;
+   cf->data[7] = bec.rxerr;
+   break;
case C_CAN_ERROR_WARNING:
/* error warning state */
cf->can_id |= CAN_ERR_CRTL;
@@ -1080,11 +1090,17 @@ static int c_can_poll(struct napi_struct *napi, int 
quota)
/* handle bus recovery events */
if ((!(curr & STATUS_BOFF)) && (last & STATUS_BOFF)) {
netdev_dbg(dev, "left bus off state\n");
-   priv->can.state = CAN_STATE_ERROR_ACTIVE;
+   work_done += c_can_handle_state_change(dev, 
C_CAN_ERROR_PASSIVE);
}
+
if ((!(curr & STATUS_EPASS)) && (last & STATUS_EPASS)) {
netdev_dbg(dev, "left error passive state\n");
-   priv->can.state = CAN_STATE_ERROR_ACTIVE;
+   work_done += c_can_handle_state_change(dev, 
C_CAN_ERROR_WARNING);
+   }
+
+   if ((!(curr & STATUS_EWARN)) && (last & STATUS_EWARN)) {
+   netdev_dbg(dev, "left error warning state\n");
+   work_done += c_can_handle_state_change(dev, C_CAN_NO_ERROR);
}
 
/* handle lec errors on the bus */
-- 
2.17.1

[PATCH] can: peakcan: report bus recovery as well

[Jeroen Hofstee]

While the state changes are reported when the error counters increase
and decrease, there is no event when the bus recovers and the error
counters decrease again. So add those as well.

Change the state going downward to be ERROR_PASSIVE -> ERROR_WARNING ->
ERROR_ACTIVE instead of directly to ERROR_ACTIVE again.

Signed-off-by: Jeroen Hofstee 
Cc: Stephane Grosjean 
---
 drivers/net/can/usb/peak_usb/pcan_usb.c | 15 ++-
 1 file changed, 10 insertions(+), 5 deletions(-)

diff --git a/drivers/net/can/usb/peak_usb/pcan_usb.c 
b/drivers/net/can/usb/peak_usb/pcan_usb.c
index 617da295b6c1..dd2a7f529012 100644
--- a/drivers/net/can/usb/peak_usb/pcan_usb.c
+++ b/drivers/net/can/usb/peak_usb/pcan_usb.c
@@ -436,8 +436,8 @@ static int pcan_usb_decode_error(struct 
pcan_usb_msg_context *mc, u8 n,
}
if ((n & PCAN_USB_ERROR_BUS_LIGHT) == 0) {
/* no error (back to active state) */
-   mc->pdev->dev.can.state = CAN_STATE_ERROR_ACTIVE;
-   return 0;
+   new_state = CAN_STATE_ERROR_ACTIVE;
+   break;
}
break;
 
@@ -460,9 +460,9 @@ static int pcan_usb_decode_error(struct 
pcan_usb_msg_context *mc, u8 n,
}
 
if ((n & PCAN_USB_ERROR_BUS_HEAVY) == 0) {
-   /* no error (back to active state) */
-   mc->pdev->dev.can.state = CAN_STATE_ERROR_ACTIVE;
-   return 0;
+   /* no error (back to warning state) */
+   new_state = CAN_STATE_ERROR_WARNING;
+   break;
}
break;
 
@@ -501,6 +501,11 @@ static int pcan_usb_decode_error(struct 
pcan_usb_msg_context *mc, u8 n,
mc->pdev->dev.can.can_stats.error_warning++;
break;
 
+   case CAN_STATE_ERROR_ACTIVE:
+   cf->can_id |= CAN_ERR_CRTL;
+   cf->data[1] = CAN_ERR_CRTL_ACTIVE;
+   break;
+
default:
/* CAN_STATE_MAX (trick to handle other errors) */
cf->can_id |= CAN_ERR_CRTL;
-- 
2.17.1

[PATCH 4/7] can: ti_hecc: keep MIM and MD set

[Jeroen Hofstee]

The HECC_CANMIM is set in the xmit path and cleared in the interrupt.
Since this is done with a read, modify, write action the register might
end up with some more MIM enabled then intended, since it is not
protected. That doesn't matter at all, since the tx interrupt disables
the mailbox with HECC_CANME (while holding a spinlock). So lets just
always keep MIM set.

While at it, since the mailbox direction never changes, don't set it
every time a message is send, ti_hecc_reset already sets them to tx.

Signed-off-by: Jeroen Hofstee 
---
 drivers/net/can/ti_hecc.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/net/can/ti_hecc.c b/drivers/net/can/ti_hecc.c
index b82c011ddbec..35c82289f2a3 100644
--- a/drivers/net/can/ti_hecc.c
+++ b/drivers/net/can/ti_hecc.c
@@ -382,6 +382,9 @@ static void ti_hecc_start(struct net_device *ndev)
hecc_set_bit(priv, HECC_CANMIM, mbx_mask);
}
 
+   /* Enable tx interrupts */
+   hecc_set_bit(priv, HECC_CANMIM, BIT(HECC_MAX_TX_MBOX) - 1);
+
/* Prevent message over-write & Enable interrupts */
hecc_write(priv, HECC_CANOPC, HECC_SET_REG);
if (priv->use_hecc1int) {
@@ -511,8 +514,6 @@ static netdev_tx_t ti_hecc_xmit(struct sk_buff *skb, struct 
net_device *ndev)
hecc_set_bit(priv, HECC_CANME, mbx_mask);
spin_unlock_irqrestore(&priv->mbx_lock, flags);
 
-   hecc_clear_bit(priv, HECC_CANMD, mbx_mask);
-   hecc_set_bit(priv, HECC_CANMIM, mbx_mask);
hecc_write(priv, HECC_CANTRS, mbx_mask);
 
return NETDEV_TX_OK;
@@ -675,7 +676,6 @@ static irqreturn_t ti_hecc_interrupt(int irq, void *dev_id)
mbx_mask = BIT(mbxno);
if (!(mbx_mask & hecc_read(priv, HECC_CANTA)))
break;
-   hecc_clear_bit(priv, HECC_CANMIM, mbx_mask);
hecc_write(priv, HECC_CANTA, mbx_mask);
spin_lock_irqsave(&priv->mbx_lock, flags);
hecc_clear_bit(priv, HECC_CANME, mbx_mask);
-- 
2.17.1

[PATCH 7/7] can: ti_hecc: add missing state changes

[Jeroen Hofstee]

While the ti_hecc has interrupts to report when the error counters increase
to a certain level and which change state it doesn't handle the case that
the error counters go down again, so the reported state can actually be
wrong. Since there is no interrupt for that, do update state based on the
error counters, when the state is not error active and goes down again.

Signed-off-by: Jeroen Hofstee 
---
 drivers/net/can/ti_hecc.c | 17 +
 1 file changed, 17 insertions(+)

diff --git a/drivers/net/can/ti_hecc.c b/drivers/net/can/ti_hecc.c
index 6098725bfdea..c7c866da9c6a 100644
--- a/drivers/net/can/ti_hecc.c
+++ b/drivers/net/can/ti_hecc.c
@@ -689,6 +689,23 @@ static irqreturn_t ti_hecc_interrupt(int irq, void *dev_id)
can_bus_off(ndev);
change_state(priv, rx_state, tx_state);
}
+   } else if (unlikely(priv->can.state != CAN_STATE_ERROR_ACTIVE)) {
+   enum can_state new_state, tx_state, rx_state;
+   u32 rec = hecc_read(priv, HECC_CANREC);
+   u32 tec = hecc_read(priv, HECC_CANTEC);
+
+   if (rec >= 128 || tec >= 128)
+   new_state = CAN_STATE_ERROR_PASSIVE;
+   else if (rec >= 96 || tec >= 96)
+   new_state = CAN_STATE_ERROR_WARNING;
+   else
+   new_state = CAN_STATE_ERROR_ACTIVE;
+
+   if (new_state < priv->can.state) {
+   rx_state = rec >= tec ? new_state : 0;
+   tx_state = rec <= tec ? new_state : 0;
+   change_state(priv, rx_state, tx_state);
+   }
}
 
if (int_status & HECC_CANGIF_GMIF) {
-- 
2.17.1

[PATCH 6/7] can: ti_hecc: properly report state changes

[Jeroen Hofstee]

The HECC_CANES register handles the flags specially, it only updates
the flags after a one is written to them. Since the interrupt for
frame errors is not enabled an old error can hence been seen when a
state interrupt arrives. For example if the device is not connected
to the CAN-bus the error warning interrupt will have HECC_CANES
indicating there is no ack. The error passive interrupt thereafter
will have HECC_CANES flagging that there is a warning level. And if
thereafter there is a message successfully send HECC_CANES points to
an error passive event, while in reality it became error warning
again. In summary, the state is not always reported correctly.

So handle the state changes and frame errors separately. The state
changes are now based on the interrupt flags and handled directly
when they occur. The reporting of the frame errors is still done as
before, as a side effect of another interrupt.

note: the hecc_clear_bit will do a read, modify, write. So it will
not only clear the bit, but also reset all other bits being set as
a side affect, hence it is replaced with only clearing the flags.

note: The HECC_CANMC_CCR is no longer cleared in the state change
interrupt, it is completely unrelated.

And use net_ratelimit to make checkpatch happy.

Signed-off-by: Jeroen Hofstee 
---
 drivers/net/can/ti_hecc.c | 156 --
 1 file changed, 82 insertions(+), 74 deletions(-)

diff --git a/drivers/net/can/ti_hecc.c b/drivers/net/can/ti_hecc.c
index 4206ad5cb666..6098725bfdea 100644
--- a/drivers/net/can/ti_hecc.c
+++ b/drivers/net/can/ti_hecc.c
@@ -149,6 +149,8 @@ MODULE_VERSION(HECC_MODULE_VERSION);
 #define HECC_BUS_ERROR (HECC_CANES_FE | HECC_CANES_BE |\
HECC_CANES_CRCE | HECC_CANES_SE |\
HECC_CANES_ACKE)
+#define HECC_CANES_FLAGS   (HECC_BUS_ERROR | HECC_CANES_BO |\
+   HECC_CANES_EP | HECC_CANES_EW)
 
 #define HECC_CANMCF_RTRBIT(4)  /* Remote transmit request */
 
@@ -578,91 +580,63 @@ static int ti_hecc_error(struct net_device *ndev, int 
int_status,
struct sk_buff *skb;
u32 timestamp;
 
-   /* propagate the error condition to the can stack */
-   skb = alloc_can_err_skb(ndev, &cf);
-   if (!skb) {
-   if (printk_ratelimit())
-   netdev_err(priv->ndev,
-  "%s: alloc_can_err_skb() failed\n",
-  __func__);
-   return -ENOMEM;
-   }
-
-   if (int_status & HECC_CANGIF_WLIF) { /* warning level int */
-   if ((int_status & HECC_CANGIF_BOIF) == 0) {
-   priv->can.state = CAN_STATE_ERROR_WARNING;
-   ++priv->can.can_stats.error_warning;
-   cf->can_id |= CAN_ERR_CRTL;
-   if (hecc_read(priv, HECC_CANTEC) > 96)
-   cf->data[1] |= CAN_ERR_CRTL_TX_WARNING;
-   if (hecc_read(priv, HECC_CANREC) > 96)
-   cf->data[1] |= CAN_ERR_CRTL_RX_WARNING;
-   }
-   hecc_set_bit(priv, HECC_CANES, HECC_CANES_EW);
-   netdev_dbg(priv->ndev, "Error Warning interrupt\n");
-   hecc_clear_bit(priv, HECC_CANMC, HECC_CANMC_CCR);
-   }
-
-   if (int_status & HECC_CANGIF_EPIF) { /* error passive int */
-   if ((int_status & HECC_CANGIF_BOIF) == 0) {
-   priv->can.state = CAN_STATE_ERROR_PASSIVE;
-   ++priv->can.can_stats.error_passive;
-   cf->can_id |= CAN_ERR_CRTL;
-   if (hecc_read(priv, HECC_CANTEC) > 127)
-   cf->data[1] |= CAN_ERR_CRTL_TX_PASSIVE;
-   if (hecc_read(priv, HECC_CANREC) > 127)
-   cf->data[1] |= CAN_ERR_CRTL_RX_PASSIVE;
+   if (err_status & HECC_BUS_ERROR) {
+   /* propagate the error condition to the can stack */
+   skb = alloc_can_err_skb(ndev, &cf);
+   if (!skb) {
+   if (net_ratelimit())
+   netdev_err(priv->ndev,
+  "%s: alloc_can_err_skb() failed\n",
+  __func__);
+   return -ENOMEM;
}
-   hecc_set_bit(priv, HECC_CANES, HECC_CANES_EP);
-   netdev_dbg(priv->ndev, "Error passive interrupt\n");
-   hecc_clear_bit(priv, HECC_CANMC, HECC_CANMC_CCR);
-   }
-
-   /* Need to check busoff condition in error status register too to
-* ensure warning interrupts don't hog the system
-*/
-   if ((int_status & HECC_CANGIF_BOIF) |

[PATCH 5/7] can: ti_hecc: add fifo underflow error reporting

[Jeroen Hofstee]

When the rx fifo overflows the ti_hecc would silently drop them since
the overwrite protection is enabled for all mailboxes. So disable it
for the lowest priority mailbox and increment the rx_fifo_errors when
receive message lost is set. Drop the message itself in that case,
since it might be partially updated.

Signed-off-by: Jeroen Hofstee 
---
 drivers/net/can/ti_hecc.c | 21 +
 1 file changed, 17 insertions(+), 4 deletions(-)

diff --git a/drivers/net/can/ti_hecc.c b/drivers/net/can/ti_hecc.c
index 35c82289f2a3..4206ad5cb666 100644
--- a/drivers/net/can/ti_hecc.c
+++ b/drivers/net/can/ti_hecc.c
@@ -82,7 +82,7 @@ MODULE_VERSION(HECC_MODULE_VERSION);
 #define HECC_CANTA 0x10/* Transmission acknowledge */
 #define HECC_CANAA 0x14/* Abort acknowledge */
 #define HECC_CANRMP0x18/* Receive message pending */
-#define HECC_CANRML0x1C/* Remote message lost */
+#define HECC_CANRML0x1C/* Receive message lost */
 #define HECC_CANRFP0x20/* Remote frame pending */
 #define HECC_CANGAM0x24/* SECC only:Global acceptance mask */
 #define HECC_CANMC 0x28/* Master control */
@@ -385,8 +385,17 @@ static void ti_hecc_start(struct net_device *ndev)
/* Enable tx interrupts */
hecc_set_bit(priv, HECC_CANMIM, BIT(HECC_MAX_TX_MBOX) - 1);
 
-   /* Prevent message over-write & Enable interrupts */
-   hecc_write(priv, HECC_CANOPC, HECC_SET_REG);
+   /* Prevent message over-write to create a rx fifo, but not for the
+* lowest priority mailbox, since that allows detecting overflows
+* instead of the hardware silently dropping the messages. The lowest
+* rx mailbox is one above the tx ones, hence its mbxno is the number
+* of tx mailboxes.
+*/
+   mbxno = HECC_MAX_TX_MBOX;
+   mbx_mask = ~BIT(mbxno);
+   hecc_write(priv, HECC_CANOPC, mbx_mask);
+
+   /* Enable interrupts */
if (priv->use_hecc1int) {
hecc_write(priv, HECC_CANMIL, HECC_SET_REG);
hecc_write(priv, HECC_CANGIM, HECC_CANGIM_DEF_MASK |
@@ -531,6 +540,7 @@ static unsigned int ti_hecc_mailbox_read(struct 
can_rx_offload *offload,
 {
struct ti_hecc_priv *priv = rx_offload_to_priv(offload);
u32 data, mbx_mask;
+   int lost;
 
mbx_mask = BIT(mbxno);
data = hecc_read_mbx(priv, mbxno, HECC_CANMID);
@@ -552,9 +562,12 @@ static unsigned int ti_hecc_mailbox_read(struct 
can_rx_offload *offload,
}
 
*timestamp = hecc_read_stamp(priv, mbxno);
+   lost = hecc_read(priv, HECC_CANRML) & mbx_mask;
+   if (unlikely(lost))
+   priv->offload.dev->stats.rx_fifo_errors++;
hecc_write(priv, HECC_CANRMP, mbx_mask);
 
-   return 1;
+   return !lost;
 }
 
 static int ti_hecc_error(struct net_device *ndev, int int_status,
-- 
2.17.1

[PATCH 3/7] can: ti_hecc: stop the CPK on down

[Jeroen Hofstee]

When the interface goes down, the CPK should no longer take an active
part in the CAN-bus communication, like sending acks and error frames.
So enable configuration mode in ti_hecc_stop, so the CPK is no longer
active.

When a transceiver switch is present the acks and errors don't make it
to the bus, but disabling the CPK then does prevent oddities, like
ti_hecc_reset failing, since the CPK can become bus-off and starts
counting the 11 bit recessive bits, which seems to block the reset. It
can also cause invalid interrupts and disrupt the CAN-bus, since
transmission can be stopped in the middle of a message, by disabling
the tranceiver while the CPK is sending.

Since the CPK is disabled after normal power on, it is typically only
seen when the interface is restarted.

Signed-off-by: Jeroen Hofstee 
---
 drivers/net/can/ti_hecc.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/net/can/ti_hecc.c b/drivers/net/can/ti_hecc.c
index 461c28ab6d66..b82c011ddbec 100644
--- a/drivers/net/can/ti_hecc.c
+++ b/drivers/net/can/ti_hecc.c
@@ -400,6 +400,9 @@ static void ti_hecc_stop(struct net_device *ndev)
 {
struct ti_hecc_priv *priv = netdev_priv(ndev);
 
+   /* Disable the CPK; stop sending, erroring and acking */
+   hecc_set_bit(priv, HECC_CANMC, HECC_CANMC_CCR);
+
/* Disable interrupts and disable mailboxes */
hecc_write(priv, HECC_CANGIM, 0);
hecc_write(priv, HECC_CANMIM, 0);
-- 
2.17.1

[PATCH 2/7] can: ti_hecc: release the mailbox a bit earlier

[Jeroen Hofstee]

Release the mailbox after reading it, so it can be reused a bit earlier.
Since "can: rx-offload: continue on error" all pending message bits are
cleared directly, so remove clearing them in ti_hecc.

Suggested-by: Marc Kleine-Budde 
Signed-off-by: Jeroen Hofstee 
---
 drivers/net/can/ti_hecc.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/drivers/net/can/ti_hecc.c b/drivers/net/can/ti_hecc.c
index f8b19eef5d26..461c28ab6d66 100644
--- a/drivers/net/can/ti_hecc.c
+++ b/drivers/net/can/ti_hecc.c
@@ -526,8 +526,9 @@ static unsigned int ti_hecc_mailbox_read(struct 
can_rx_offload *offload,
 u32 *timestamp, unsigned int mbxno)
 {
struct ti_hecc_priv *priv = rx_offload_to_priv(offload);
-   u32 data;
+   u32 data, mbx_mask;
 
+   mbx_mask = BIT(mbxno);
data = hecc_read_mbx(priv, mbxno, HECC_CANMID);
if (data & HECC_CANMID_IDE)
cf->can_id = (data & CAN_EFF_MASK) | CAN_EFF_FLAG;
@@ -547,6 +548,7 @@ static unsigned int ti_hecc_mailbox_read(struct 
can_rx_offload *offload,
}
 
*timestamp = hecc_read_stamp(priv, mbxno);
+   hecc_write(priv, HECC_CANRMP, mbx_mask);
 
return 1;
 }
@@ -695,7 +697,6 @@ static irqreturn_t ti_hecc_interrupt(int irq, void *dev_id)
while ((rx_pending = hecc_read(priv, HECC_CANRMP))) {
can_rx_offload_irq_offload_timestamp(&priv->offload,
 rx_pending);
-   hecc_write(priv, HECC_CANRMP, rx_pending);
}
}
 
-- 
2.17.1

[PATCH 1/7] can: rx-offload: continue on error

[Jeroen Hofstee]

While can_rx_offload_offload_one will call mailbox_read to discard
the mailbox in case of an error, can_rx_offload_irq_offload_timestamp
bails out in the error case. Since it is typically called from a while
loop, all message will eventually be discarded. So lets continue on
error instead to discard them directly.

Signed-off-by: Jeroen Hofstee 
---
 drivers/net/can/rx-offload.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/can/rx-offload.c b/drivers/net/can/rx-offload.c
index e6a668ee7730..39df41280e2d 100644
--- a/drivers/net/can/rx-offload.c
+++ b/drivers/net/can/rx-offload.c
@@ -158,7 +158,7 @@ int can_rx_offload_irq_offload_timestamp(struct 
can_rx_offload *offload, u64 pen
 
skb = can_rx_offload_offload_one(offload, i);
if (!skb)
-   break;
+   continue;
 
__skb_queue_add_sort(&skb_queue, skb, can_rx_offload_compare);
}
-- 
2.17.1

Re: [PATCH] can: ti_hecc: use timestamp based rx-offloading

[Jeroen Hofstee]

Hello Marc,

On 7/26/19 1:48 PM, Marc Kleine-Budde wrote:
> On 4/29/19 2:03 PM, Jeroen Hofstee wrote:
>
>> @@ -744,8 +652,8 @@ static irqreturn_t ti_hecc_interrupt(int irq, void 
>> *dev_id)
>>  struct net_device *ndev = (struct net_device *)dev_id;
>>  struct ti_hecc_priv *priv = netdev_priv(ndev);
>>  struct net_device_stats *stats = &ndev->stats;
>> -u32 mbxno, mbx_mask, int_status, err_status;
>> -unsigned long ack, flags;
>> +u32 mbxno, mbx_mask, int_status, err_status, stamp;
>> +unsigned long flags, rx_pending;
>>   
>>  int_status = hecc_read(priv,
>>  (priv->use_hecc1int) ? HECC_CANGIF1 : HECC_CANGIF0);
>> @@ -769,11 +677,11 @@ static irqreturn_t ti_hecc_interrupt(int irq, void 
>> *dev_id)
>>  spin_lock_irqsave(&priv->mbx_lock, flags);
>>  hecc_clear_bit(priv, HECC_CANME, mbx_mask);
>>  spin_unlock_irqrestore(&priv->mbx_lock, flags);
>> -stats->tx_bytes += hecc_read_mbx(priv, mbxno,
>> -HECC_CANMCF) & 0xF;
>> +stamp = hecc_read_stamp(priv, mbxno);
>> +stats->tx_bytes += 
>> can_rx_offload_get_echo_skb(&priv->offload,
>> +mbxno, 
>> stamp);
>>  stats->tx_packets++;
>>  can_led_event(ndev, CAN_LED_EVENT_TX);
>> -can_get_echo_skb(ndev, mbxno);
>>  --priv->tx_tail;
>>  }
>>   
>> @@ -784,12 +692,11 @@ static irqreturn_t ti_hecc_interrupt(int irq, void 
>> *dev_id)
>>  ((priv->tx_head & HECC_TX_MASK) == HECC_TX_MASK)))
>>  netif_wake_queue(ndev);
>>   
>> -/* Disable RX mailbox interrupts and let NAPI reenable them */
>> -if (hecc_read(priv, HECC_CANRMP)) {
>> -ack = hecc_read(priv, HECC_CANMIM);
>> -ack &= BIT(HECC_MAX_TX_MBOX) - 1;
>> -hecc_write(priv, HECC_CANMIM, ack);
>> -napi_schedule(&priv->napi);
>> +/* offload RX mailboxes and let NAPI deliver them */
>> +while ((rx_pending = hecc_read(priv, HECC_CANRMP))) {
>> +can_rx_offload_irq_offload_timestamp(&priv->offload,
>> + rx_pending);
>> +hecc_write(priv, HECC_CANRMP, rx_pending);
> Can prepare a patch to move the RMP writing into the mailbox_read()
> function. This makes the mailbox available a bit earlier and works
> better for memory pressure situations, where no skb can be allocated.


For my understanding, is there any other reason for alloc_can_skb,
to fail, besides being out of memory. I couldn't easily find an other
limit enforced on it.

If it is actually _moved_, as you suggested, it does loose the ability to
handle the newly received messages while the messages are read
in the same interrupt, so it needs to interrupt again. That will work,
but seems a bit a silly thing to do.

Perhaps we can do both? Mark the mailbox as available in
mailbox_read, so it is available as soon as possible and clear
them all in the irq (under the assumption that alloc_can_skb
failure means real big trouble, why would we want to keep the
old messages around and eventually ignore the new messages?).

Another question, not related to this patch, but this driver..
Most of the times the irq handles 1 or sometimes 2 messages.
Do you happen to know if it is possible to optionally delay the irq
a bit in the millisecond range, so more work can be done in a single
interrupt? Since there are now 28 rx hardware mailboxes available,
it shouldn't run out easily.

And as last, I guess you want a patch which applies to
linux-can-next/testing, right?

With kind regards,

Jeroen

Re: [PATCH] can: ti_hecc: use timestamp based rx-offloading

[Jeroen Hofstee]

Hello Marc,

On 7/24/19 10:26 AM, Marc Kleine-Budde wrote:
> On 4/29/19 2:03 PM, Jeroen Hofstee wrote:
>> As already mentioned in [1] and included in [2], there is an off by one
>> issue since the high bank is already enabled when the _next_ mailbox to
>> be read has index 12, so the mailbox being read was 13. The message can
>> therefore go into mailbox 31 and the driver will be repolled until the
>> mailbox 12 eventually receives a msg. Or the message might end up in the
>> 12th mailbox, but then it would become disabled after reading it and only
>> be enabled again in the next "round" after mailbox 13 was read, which can
>> cause out of order messages, since the lower priority mailboxes can
>> accept messages in the meantime.
>>
>> As mentioned in [3] there is a hardware race condition when changing the
>> CANME register while messages are being received. Even when including a
>> busy poll on reception, like in [2] there are still overflows and out of
>> order messages at times, but less then without the busy loop polling.
>> Unlike what the patch suggests, the polling time is not in the microsecond
>> range, but takes as long as a current CAN bus reception needs to finish,
>> so typically more in the fraction of millisecond range. Since the timeout
>> is in jiffies it won't timeout.
>>
>> Even with these additional fixes the driver is still not able to provide a
>> proper FIFO which doesn't drop packages. So change the driver to use
>> rx-offload and base order on timestamp instead of message box numbers. As
>> a side affect, this also fixes [4] and [5].
>>
>> Before this change messages with a single byte counter were dropped /
>> received out of order at a bitrate of 250kbit/s on an am3517. With this
>> patch that no longer occurs up to and including 1Mbit/s.
>>
>> [1] 
>> https://linux-can.vger.kernel.narkive.com/zgO9inVi/patch-can-ti-hecc-fix-rx-wrong-sequence-issue#post6
>> [2] 
>> http://arago-project.org/git/projects/?p=linux-omap3.git;a=commit;h=02346892777f07245de4d5af692513ebd852dcb2
>> [3] 
>> https://linux-can.vger.kernel.narkive.com/zgO9inVi/patch-can-ti-hecc-fix-rx-wrong-sequence-issue#post5
>> [4] https://patchwork.ozlabs.org/patch/895956/
>> [5] https://www.spinics.net/lists/netdev/msg494971.html
>>
>> Cc: Anant Gole 
>> Cc: AnilKumar Ch 
>> Signed-off-by: Jeroen Hofstee 
>> ---
>>   drivers/net/can/ti_hecc.c | 189 
>> +-
>>   1 file changed, 53 insertions(+), 136 deletions(-)
>>
>> diff --git a/drivers/net/can/ti_hecc.c b/drivers/net/can/ti_hecc.c
>> index db6ea93..fe7 100644
>> --- a/drivers/net/can/ti_hecc.c
>> +++ b/drivers/net/can/ti_hecc.c
>> @@ -5,6 +5,7 @@
>>* specs for the same is available at <http://www.ti.com>
>>*
>>* Copyright (C) 2009 Texas Instruments Incorporated - http://www.ti.com/
>> + * Copyright (C) 2019 Jeroen Hofstee 
>>*
>>* This program is free software; you can redistribute it and/or
>>* modify it under the terms of the GNU General Public License as
>> @@ -34,6 +35,7 @@
>>   #include 
>>   #include 
>>   #include 
>> +#include 
>>   
>>   #define DRV_NAME "ti_hecc"
>>   #define HECC_MODULE_VERSION "0.7"
>> @@ -63,29 +65,16 @@ MODULE_VERSION(HECC_MODULE_VERSION);
>>   #define HECC_TX_PRIO_MASK  (MAX_TX_PRIO << HECC_MB_TX_SHIFT)
>>   #define HECC_TX_MB_MASK(HECC_MAX_TX_MBOX - 1)
>>   #define HECC_TX_MASK   ((HECC_MAX_TX_MBOX - 1) | 
>> HECC_TX_PRIO_MASK)
>> -#define HECC_TX_MBOX_MASK   (~(BIT(HECC_MAX_TX_MBOX) - 1))
>> -#define HECC_DEF_NAPI_WEIGHTHECC_MAX_RX_MBOX
>>   
>>   /*
>> - * Important Note: RX mailbox configuration
>> - * RX mailboxes are further logically split into two - main and buffer
>> - * mailboxes. The goal is to get all packets into main mailboxes as
>> - * driven by mailbox number and receive priority (higher to lower) and
>> - * buffer mailboxes are used to receive pkts while main mailboxes are being
>> - * processed. This ensures in-order packet reception.
>> - *
>> - * Here are the recommended values for buffer mailbox. Note that RX 
>> mailboxes
>> - * start after TX mailboxes:
>> - *
>> - * HECC_MAX_RX_MBOX HECC_RX_BUFFER_MBOX No of buffer mailboxes
>> - * 28   12  8
>> - * 16   20  4
>> + * RX mailbox configuration
>> + * The remaining ma

[PATCH] can: ti_hecc: fix close when napi poll is active

[Jeroen Hofstee]

When closing this CAN interface while napi poll is active, for example with:
`ip link set can0 down` several interfaces freeze. This seemed to be caused
by napi_disable called from ti_hecc_close expecting the scheduled probe to
either return quota or call napi_complete. Since the poll functions has a
check for netif_running it returns 0 and doesn't call napi_complete and hence
violates the napi its expectation.

So remove this check, so either napi_complete is called or quota is returned.

Signed-off-by: Jeroen Hofstee 
---
 drivers/net/can/ti_hecc.c | 3 ---
 1 file changed, 3 deletions(-)

diff --git a/drivers/net/can/ti_hecc.c b/drivers/net/can/ti_hecc.c
index db6ea93..42813d3 100644
--- a/drivers/net/can/ti_hecc.c
+++ b/drivers/net/can/ti_hecc.c
@@ -603,9 +603,6 @@ static int ti_hecc_rx_poll(struct napi_struct *napi, int 
quota)
u32 mbx_mask;
unsigned long pending_pkts, flags;
 
-   if (!netif_running(ndev))
-   return 0;
-
while ((pending_pkts = hecc_read(priv, HECC_CANRMP)) &&
num_pkts < quota) {
mbx_mask = BIT(priv->rx_next); /* next rx mailbox to process */
-- 
2.7.4

Re: [PATCH v2] net: cpsw: fix obtaining mac address for am3517

[Jeroen Hofstee]

Hello Tony,

On 28-10-16 17:52, Tony Lindgren wrote:

* Jeroen Hofstee  [161028 08:33]:

Commit b6745f6e4e63 ("drivers: net: cpsw: davinci_emac: move reading mac
id to common file") did not only move the code for an am3517, it also
added the slave parameter, resulting in an invalid (all zero) mac address
being returned for an am3517, since it only has a single emac and the slave
parameter is pointing to the second. So simply always read the first and
valid mac-address for a ti,am3517-emac.

And others davinci_emac.c users can have more than one. So is the
reason the slave parameter points to the second instance because
of the location in the hardware?


Sort of, the slave parameter gets determined by the fact if there is one
or two register range(s) associated with the davinci_emac. In davinci_emac.c

res_ctrl = platform_get_resource(pdev, IORESOURCE_MEM, 1);
...
rc = davinci_emac_try_get_mac(pdev, res_ctrl ? 0 : 1,
  priv->mac_addr);

So it there are two ranges, the slave param becomes 0. It there is only 
one, it
will be 1. Since the am3517 only has a single regs entry it ends up with 
slave 1,

while there is only a single davinci_emac.

Regards,
Jeroen

arch/arm/boot/dts/dm816x.dtsi
-
eth0: ethernet@4a10 {
compatible = "ti,dm816-emac";
ti,hwmods = "emac0";
reg = <0x4a10 0x800
   0x4a100900 0x3700>;
clocks = <&sysclk24_ck>;
syscon = <&scm_conf>;
ti,davinci-ctrl-reg-offset = <0>;
ti,davinci-ctrl-mod-reg-offset = <0x900>;
ti,davinci-ctrl-ram-offset = <0x2000>;
ti,davinci-ctrl-ram-size = <0x2000>;
interrupts = <40 41 42 43>;
phy-handle = <&phy0>;
};

eth1: ethernet@4a12 {
compatible = "ti,dm816-emac";
ti,hwmods = "emac1";
reg = <0x4a12 0x4000>;
clocks = <&sysclk24_ck>;
syscon = <&scm_conf>;
ti,davinci-ctrl-reg-offset = <0>;
ti,davinci-ctrl-mod-reg-offset = <0x900>;
ti,davinci-ctrl-ram-offset = <0x2000>;
ti,davinci-ctrl-ram-size = <0x2000>;
interrupts = <44 45 46 47>;
phy-handle = <&phy1>;
};

arch/arm/boot/dts/am3517.dtsi
---
davinci_emac: ethernet@0x5c00 {
compatible = "ti,am3517-emac";
ti,hwmods = "davinci_emac";
status = "disabled";
reg = <0x5c00 0x3>;
interrupts = <67 68 69 70>;
syscon = <&scm_conf>;
ti,davinci-ctrl-reg-offset = <0x1>;
ti,davinci-ctrl-mod-reg-offset = <0>;
ti,davinci-ctrl-ram-offset = <0x2>;
ti,davinci-ctrl-ram-size = <0x2000>;
ti,davinci-rmii-en = /bits/ 8 <1>;
local-mac-address = [ 00 00 00 00 00 00 ];
};

Re: [PATCH] net: cpsw: fix obtaining mac address for am3517

[Jeroen Hofstee]

Hello Tony,


On 21-10-16 09:53, Tony Lindgren wrote:

* Jeroen Hofstee  [161021 00:37]:

Hello Tony,

On 21-10-16 08:38, Tony Lindgren wrote:

* Jeroen Hofstee  [161020 12:57]:

Commit b6745f6e4e63 ("drivers: net: cpsw: davinci_emac: move reading mac
id to common file") did not only move the code for an am3517, it also
added the slave parameter, resulting in a invalid (all zero) mac address
being returned. So change it back to always read from slave zero, so it
works again.

Hmm doesn't this now break it for cpsw with two instances?


Yes, well, they get the same mac address at least. But does it matter?
This changes davinci_emac_3517_get_macid, the only way to get there
is:

 if (of_device_is_compatible(dev->of_node, "ti,am3517-emac"))
 return davinci_emac_3517_get_macid(dev, 0x110, slave, mac_addr)

and the only user of ti,am3517-emac is arch/arm/boot/dts/am3517.dtsi,
which only has one emac. So the change is already am3517 specific.


We may need am3517 specific quirk flag instead?

Given above, it is already am3517 specific. Let me know if you prefer this
route then I will have a look at it.

Oh OK thanks for explaining it :) As it's already am3517 specific:

Acked-by: Tony Lindgren 


Aaah, lets wait a sec. I just saw there is another user of this function,
so above is simply not true

if (of_machine_is_compatible("ti,dra7"))
return davinci_emac_3517_get_macid(dev, 0x514, slave, mac_addr);


So let me check if I don't break that one. or better, let me send a 
v2, which

changes the caller to pass slave as 0 here?

if (of_device_is_compatible(dev->of_node, "ti,am3517-emac"))
return davinci_emac_3517_get_macid(dev, 0x110, slave, mac_addr);

Regards,
Jeroen

Re: [PATCH] net: cpsw: fix obtaining mac address for am3517

[Jeroen Hofstee]

Hello Tony,

On 21-10-16 08:38, Tony Lindgren wrote:

* Jeroen Hofstee  [161020 12:57]:

Commit b6745f6e4e63 ("drivers: net: cpsw: davinci_emac: move reading mac
id to common file") did not only move the code for an am3517, it also
added the slave parameter, resulting in a invalid (all zero) mac address
being returned. So change it back to always read from slave zero, so it
works again.

Hmm doesn't this now break it for cpsw with two instances?



Yes, well, they get the same mac address at least. But does it matter?
This changes davinci_emac_3517_get_macid, the only way to get there
is:

if (of_device_is_compatible(dev->of_node, "ti,am3517-emac"))
return davinci_emac_3517_get_macid(dev, 0x110, slave, mac_addr)

and the only user of ti,am3517-emac is arch/arm/boot/dts/am3517.dtsi,
which only has one emac. So the change is already am3517 specific.


We may need am3517 specific quirk flag instead?


Given above, it is already am3517 specific. Let me know if you prefer this
route then I will have a look at it.

Regards,
Jeroen

Re: Device Tree Blob (DTB) licence

[Jeroen Hofstee]

Hi,

On 22-05-15 12:05, Yann Droneaud wrote:

Le mardi 05 mai 2015 à 11:41 -0500, Rob Herring a écrit :

On Tue, May 5, 2015 at 5:05 AM, Yann Droneaud 
wrote:

I believe Device Tree Blob (.dtb file) built from kernel's Device
Tree
Sources (.dts, which #include .dtsi, which #include .h) using
Device
Tree Compiler (dtc) are covered by GNU General Public Licence v2
(GPLv2), but cannot find any reference.

By default yes, but we've been steering people to dual license them
GPL/BSD.





obviously these files should be reusable. If there is a license issue
with that it should be fixed. cc-ing freebsd-...@freebsd.org.

I am not a lawyer,

Jeroen
--
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: [U-Boot] [PATCH resend] kconfig: Fix compiler warning in menu.c

[Jeroen Hofstee]

Hello Simon,

On 13-10-14 07:14, Simon Glass wrote:

Hi Jeroen,

On 12 October 2014 10:13, Jeroen Hofstee  wrote:


Hello Hans,

On 12-10-14 12:25, Hans de Goede wrote:


Hi,

This one seems to have fallen through the cracks.

Regards,

Hans

  (for U-boot)

nope, you replace an innocent warning (_might_ be) with
bad code, without any comment it is just because gcc failed
to recognize it is fine. Nor did you respond to the suggestion
if it helps gcc to recognize that if the two booleans are merged
into a single one. [or even split it in an if () if ()]. With this patch
you prevent any serious warning in case the variable is actually
used but not initialized, which is even worse if you ask me.


That is a pretty acerbic tone to take on the U-Boot list at least. Are you
two drinking buddies or something?


no, it is because we have discussed this patch before and resending
it won't address the issue raised. But you are right, it is likely done with
less evil intends then I took it for, so let me explain my concern again
in a politer way. The problem is that gcc 4.9 starts warning in the
following case:

int *ptr;

if (a)
ptr = something;

if (a && b)
ptr->bla = value;
else
   do_something_else();


it will warn that ptr _might_ be used uninitialized (but it always is).
This is fixed in this patch by assigning NULL to ptr, and while that makes
the warning go away it actually prevents the valid warning, ptr _is_ used
uninitialized if you start using it in the else case. Hence my request if we
can't find a better solution for this.

Does anyone know a better solution for this or should we consider
disabling the might be unused warning?

Regards,
Jeroen

--
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: [U-Boot] [PATCH resend] kconfig: Fix compiler warning in menu.c

[Jeroen Hofstee]

Hello Hans,

On 12-10-14 12:25, Hans de Goede wrote:

Hi,

This one seems to have fallen through the cracks.

Regards,

Hans


(for U-boot)

nope, you replace an innocent warning (_might_ be) with
bad code, without any comment it is just because gcc failed
to recognize it is fine. Nor did you respond to the suggestion
if it helps gcc to recognize that if the two booleans are merged
into a single one. [or even split it in an if () if ()]. With this patch
you prevent any serious warning in case the variable is actually
used but not initialized, which is even worse if you ask me.

Regards,
Jeroen


--
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: [U-Boot] [PATCH] kconfig: Fix compiler warning in menu.c

[Jeroen Hofstee]

Hello Hans,

On 31-07-14 16:21, Hans de Goede wrote:

This fixes the following compiler warning:

In file included from scripts/kconfig/zconf.tab.c:2537:0:
scripts/kconfig/menu.c: In function ‘get_symbol_str’:
scripts/kconfig/menu.c:590:18: warning: ‘jump’ may be used uninitialized in 
this function [-Wmaybe-uninitialized]
  jump->offset = strlen(r->s);
   ^
In file included from scripts/kconfig/zconf.tab.c:2537:0:
scripts/kconfig/menu.c:551:19: note: ‘jump’ was declared here
   struct jump_key *jump;

Signed-off-by: Hans de Goede 
---
  scripts/kconfig/menu.c | 2 +-
  1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/scripts/kconfig/menu.c b/scripts/kconfig/menu.c
index a26cc5d..584e0fc 100644
--- a/scripts/kconfig/menu.c
+++ b/scripts/kconfig/menu.c
@@ -548,7 +548,7 @@ static void get_prompt_str(struct gstr *r, struct property 
*prop,
  {
int i, j;
struct menu *submenu[8], *menu, *location = NULL;
-   struct jump_key *jump;
+   struct jump_key *jump = NULL;
  
  	str_printf(r, _("Prompt: %s\n"), _(prop->text));

menu = prop->menu->parent;


And if you combine head && location into a single boolean,
does this warning still occur? Not that it matters that much
in this case, since it is a host tool, but I guess the compiler
does intend to assign NULL while there is no reason to do so.

Regards,
Jeroen
--
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: [U-Boot] [PATCH] kconfig: Fix compiler warning in menu.c

[Jeroen Hofstee]

Hello Hans,

On 31-07-14 16:21, Hans de Goede wrote:

This fixes the following compiler warning:

In file included from scripts/kconfig/zconf.tab.c:2537:0:
scripts/kconfig/menu.c: In function ‘get_symbol_str’:
scripts/kconfig/menu.c:590:18: warning: ‘jump’ may be used uninitialized in 
this function [-Wmaybe-uninitialized]
  jump->offset = strlen(r->s);
   ^
In file included from scripts/kconfig/zconf.tab.c:2537:0:
scripts/kconfig/menu.c:551:19: note: ‘jump’ was declared here
   struct jump_key *jump;

Signed-off-by: Hans de Goede 
---
  scripts/kconfig/menu.c | 2 +-
  1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/scripts/kconfig/menu.c b/scripts/kconfig/menu.c
index a26cc5d..584e0fc 100644
--- a/scripts/kconfig/menu.c
+++ b/scripts/kconfig/menu.c
@@ -548,7 +548,7 @@ static void get_prompt_str(struct gstr *r, struct property 
*prop,
  {
int i, j;
struct menu *submenu[8], *menu, *location = NULL;
-   struct jump_key *jump;
+   struct jump_key *jump = NULL;
  
  	str_printf(r, _("Prompt: %s\n"), _(prop->text));

menu = prop->menu->parent;


just curious, which compiler is this? Since it is a false positive.
(and it could know)

Regards,
Jeroen
--
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: [U-Boot] [RFC] kbuild.h: workaround for llvm IAS

[Jeroen Hofstee]

Hello Masahiro,

On 27-06-14 08:38, Masahiro Yamada wrote:

KBuild (ab)uses the asm statement to write to a file and
llvm integrated as chokes about these invalid asm statements.
Workaround it by making it look like valid asm code.

Signed-off-by: Jeroen Hofstee 

I think Linux has the same problem.

Are you willing to this patch to linux-kbuild ML?
Or fixing U-Boot only?

I don't mind in general, but it is just noise for them (cc-ing them to
create some).  For u-boot (ARM) you actually get a valid binary with
this patch after clang support has landed, for linux you just get other
errors as far as I tried (native only), patch below.

However in linux there seem more spots relying on the format, e.g.
  arch/ia64/kvm/Makefile
  arch/ia64/kernel/Makefile
  arch/um/Makefile

So if anything, I think this should be made a general rules first
in the makefiles. It seems stupid to potentially break something
while it gains nothing.

So yes, u-boot only afaic, or does that make your syncing more difficult?

I don't think syncing would be difficult.

BTW, do you know how they resolve this build error in other projects,
for example, in llvmlinux ?
http://llvm.linuxfoundation.org/index.php/Main_Page

Linux folks merged Clang support into the top Makefile, but not into ./Kbuild.
I don't know why.

I don't know how the llvmlinux people do it, but the alternative is to
add -no-integrated-as for clang when compiling such files (or use an
older clang version, since that used to be the default). Since gcc's LTO
dislikes the asm-offset.c technique as well, I think it is better to 
actually

create valid asm, so it no longer depends on compiler features at all.
I will leave it up to the llvmlinux folks to come up with a solution for
linux though...

Regards,
Jeroen





--
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: [U-Boot] [RFC] kbuild.h: workaround for llvm IAS

[Jeroen Hofstee]

Hi Masahiro,

On 24-06-14 14:53, Masahiro Yamada wrote:

On Thu, 12 Jun 2014 23:40:54 +0200
Jeroen Hofstee  wrote:


KBuild (ab)uses the asm statement to write to a file and
llvm integrated as chokes about these invalid asm statements.
Workaround it by making it look like valid asm code.

Signed-off-by: Jeroen Hofstee 

I think Linux has the same problem.

Are you willing to this patch to linux-kbuild ML?
Or fixing U-Boot only?

I don't mind in general, but it is just noise for them (cc-ing them to
create some).  For u-boot (ARM) you actually get a valid binary with
this patch after clang support has landed, for linux you just get other
errors as far as I tried (native only), patch below.

However in linux there seem more spots relying on the format, e.g.
arch/ia64/kvm/Makefile
arch/ia64/kernel/Makefile
arch/um/Makefile

So if anything, I think this should be made a general rules first
in the makefiles. It seems stupid to potentially break something
while it gains nothing.

So yes, u-boot only afaic, or does that make your syncing more difficult?

Regards,
Jeroen


--- a/Kbuild
+++ b/Kbuild
@@ -52,7 +52,8 @@ targets += arch/$(SRCARCH)/kernel/asm-offsets.s

 # Default sed regexp - multiline due to syntax constraints
 define sed-y
-   "/^->/{s:->#\(.*\):/* \1 */:; \
+   "s:[[:space:]]*\.ascii[[:space:]]*\"\(.*\)\":\1:; \
+   /^->/{s:->#\(.*\):/* \1 */:; \
s:^->\([^ ]*\) [\$$#]*\([-0-9]*\) \(.*\):#define \1 \2 /* \3 */:; \
s:^->\([^ ]*\) [\$$#]*\([^ ]*\) \(.*\):#define \1 \2 /* \3 */:; \
s:->::; p;}"
diff --git a/include/linux/kbuild.h b/include/linux/kbuild.h
index 22a7219..4e80f3a 100644
--- a/include/linux/kbuild.h
+++ b/include/linux/kbuild.h
@@ -2,14 +2,14 @@
 #define __LINUX_KBUILD_H

 #define DEFINE(sym, val) \
-asm volatile("\n->" #sym " %0 " #val : : "i" (val))
+   asm volatile("\n.ascii \"->" #sym " %0 " #val "\"" : : "i" (val))

-#define BLANK() asm volatile("\n->" : : )
+#define BLANK() asm volatile("\n.ascii \"->\"" : : )

 #define OFFSET(sym, str, mem) \
DEFINE(sym, offsetof(struct str, mem))

 #define COMMENT(x) \
-   asm volatile("\n->#" x)
+   asm volatile("\n.ascii \"->#" x "\"")

 #endif
diff --git a/scripts/mod/Makefile b/scripts/mod/Makefile
index c11212f..0698af3 100644
--- a/scripts/mod/Makefile
+++ b/scripts/mod/Makefile
@@ -6,7 +6,8 @@ modpost-objs:= modpost.o file2alias.o sumversion.o
 devicetable-offsets-file := devicetable-offsets.h

 define sed-y
-   "/^->/{s:->#\(.*\):/* \1 */:; \
+   "s:[[:space:]]*\.ascii[[:space:]]*\"\(.*\)\":\1:; \
+   /^->/{s:->#\(.*\):/* \1 */:; \
s:^->\([^ ]*\) [\$$#]*\([-0-9]*\) \(.*\):#define \1 \2 /* \3 */:; \
s:^->\([^ ]*\) [\$$#]*\([^ ]*\) \(.*\):#define \1 \2 /* \3 */:; \
s:->::; p;}"
--
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/

Invalid checksum calculated for ipv4 SYN+ACK when opt.srr is set?

[Jeroen van Bemmel]

Hi,

Although rarely used and considered plain evil by some, the kernel
code allows one to use IP source routing for TCP.

In tcp_v4_send_synack the following two lines of code are used:
[...]
__tcp_v4_send_check(skb, ireq->loc_addr, ireq->rmt_addr);
[...]
err = ip_build_and_send_pkt(skb, sk, ireq->loc_addr,ireq->rmt_addr,ireq->opt);

In ip_build_and_send_pkt the 'daddr' parameter is used as follows:
iph->daddr = (opt && opt->opt.srr ? opt->opt.faddr : daddr);

It looks like the checksum may be off when (opt && opt->opt.srr ) is true?

Thanks,
Jeroen
--
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: Link state change detection problem on Moschip MCS7832 again

[Jeroen Koekkoek]

On Wed, 2013-07-31 at 11:11 +0200, Jeroen Koekkoek wrote:
> On Fri, 2013-01-11 at 11:15 +0100, Michael Hunold wrote:
> > Hi,
> > 
> > I have a no-name Moschip MCS7832-based adapter shows a strange behaviour 
> > in my system after a system upgrade. "lsusb -vv" for that device is 
> > attached to the end of the mail.
> > 
> > I am using the adapter for embedded systems development, where it serves 
> > kernels via TFTP and root filesystems via NFS.
> > 
> > I have recently upgrade my system to Kubuntu 12.10 which uses a 3.5.0-21 
> > kernel. Before that upgrade the device was working fine with Xubuntu 10.10.
> > 
> > I have used the network-manager applet that comes with Kubuntu to assign 
> > a static IP address to that interface.
> > 
> > The symptom is that when the remote system's bootloader (u-boot in my 
> > case) starts to fetch the kernel via TFTP, it usually starts fine (a 
> > couple of "#" are shown to indicate progress), then timeouts are 
> > happening ("T" is shown), then progress continues, then more timeouts 
> > and so on.
> > 
> > I can see the following messages getting repeated in /var/log/syslog:
> > 
> > [...]
> > Jan 11 11:01:04 elmc-teemhu NetworkManager[1250]:  (eth1): carrier 
> > now OFF (device state 100, deferring action for 4 seconds)
> > Jan 11 11:01:04 elmc-teemhu NetworkManager[1250]:  (eth1): carrier 
> > now ON (device state 100)
> > [...]
> > 
> > I found the following bug report and this got me going:
> > https://bugzilla.kernel.org/show_bug.cgi?id=28532
> > 
> > Here is what I investigated so far.
> > 
> > 1. I noticed that the patch dabdaf0caa3af520dbc1df87b2fb4e77224037bd 
> > from Ondrej Zary is missing in the kernel Kubuntu is serving, so I 
> > downloaded the most-recent mcs7830.c from kernel.org and recompiled the 
> > module. The problem stays the same, there is no improvement.
> > 
> > 2. I undid both commits dabdaf0caa3af520dbc1df87b2fb4e77224037bd and 
> > b1ff4f96fd1c63890d78d8939c6e0f2b44ce3113 which added the "mcs7830: 
> > Implement link state detection" in the first place. Without that 
> > "feature" my adapter is now working reliably again.
> > 
> > 3. Commit dabdaf0caa3af520dbc1df87b2fb4e77224037bd had the following 
> > comment:
> > 
> > "The device had an undocumented "feature": it can provide a sequence of
> > spurious link-down status data even if the link is up all the time.
> > A sequence of 10 was seen so update the link state only after the device
> > reports the same link state 20 times."
> > 
> > I tried to increase the number from 20 gradually, but it did not help to 
> > fix the problem. In my desparation I tried 100 as well, but this only 
> > postponed the
> > 
> > 4. In my desparation, I went back to the most recent driver and added 
> > the following code to mcs7830_status() in order to track after how many 
> > calls to that function the link state changes.
> > 
> > [...]
> > {
> > static int xxx_counter = 0;
> > static int xxx_link = -1;
> > if (link != xxx_link) {
> > printk("counter %4d -> link %d\n", xxx_counter, link);
> > xxx_link = link;
> > xxx_counter = 0;
> > } else {
> > xxx_counter++;
> > }
> > }
> > [...]
> > 
> > This resulted in the following output:
> > 
> > Jan 11 11:01:04 elmc-teemhu kernel: [11627.025109] counter  105 -> link 0
> > Jan 11 11:01:04 elmc-teemhu kernel: [11627.101840] counter   76 -> link 1
> > Jan 11 11:01:04 elmc-teemhu kernel: [11627.207724] counter  105 -> link 0
> > Jan 11 11:01:04 elmc-teemhu kernel: [11627.285582] counter   77 -> link 1
> > Jan 11 11:01:04 elmc-teemhu kernel: [11627.392416] counter  106 -> link 0
> > Jan 11 11:01:04 elmc-teemhu kernel: [11627.468149] counter   75 -> link 1
> > Jan 11 11:01:04 elmc-teemhu kernel: [11627.574036] counter  105 -> link 0
> > Jan 11 11:01:04 elmc-teemhu kernel: [11627.651893] counter   77 -> link 1
> > Jan 11 11:01:04 elmc-teemhu kernel: [11627.757719] counter  105 -> link 0
> > Jan 11 11:01:04 elmc-teemhu NetworkManager[1250]:  (eth1): carrier 
> > now OFF (device state 100, deferring action for 4 seconds)
> > Jan 11 11:01:04 elmc-teemhu kernel: [11627.834546] counter   76 -> link 1
> > Jan 11 11:01:04 elmc-teemhu NetworkManager[1250]:  (eth1): carrier 
> > now ON (device state 100)
> > Jan 11 11:01:05 elmc-teemhu kernel

Re: Link state change detection problem on Moschip MCS7832 again

[Jeroen Koekkoek]

and NFS is working fine.
> 
> Any idea what is wrong with that adapter? Is it unable to report link 
> state changes correctly at all?
> 
> How to make the current driver work correctly without reverting the two 
> commit completly?
> 
> Best regards
> Michael.

Hi,

I'm experiencing the same problem. After reverting the commits below, my
problems disappeared.

commit b1ff4f96fd1c63890d78d8939c6e0f2b44ce3113
mcs7830: Implement link state detection

commit b1ff4f96fd1c63890d78d8939c6e0f2b44ce3113
mcs7830: Implement link state detection

I'm running Fedora 19 (Linux laptop.lan 3.10.3-300.fc19.x86_64 #1 SMP
Fri Jul 26 00:00:58 UTC 2013 x86_64 x86_64 x86_64 GNU/Linux).

Best regards,
Jeroen Koekkoek

--
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/

What is the use of RLIMIT_NICE ?

[Jeroen Ooms]

I am using prlimit in Ubuntu to do some resource restrictions in my
sandbox which has been very helpful. However, I am not quite sure what
to do with RLIMIT_NICE. The docs say:

RLIMIT_NICE: Specifies a ceiling to which the process's nice value can
be raised using setpriority(2) or nice(2).

However, according to getpriority(2), a process can raise it's nice
value only if owned by a superuser in the first place. But if this is
the case, the RLIMIT_NICE value is not going to add too any
functionality because a privileged user can arbitrarily lower or
higher RLIMIT values anyway.

So I don't understand how to use or interpret RLIMIT_NICE. For
non-privileged users the entire thing seems useless because they
cannot raise priority in the first place, and it makes no sense to set
it below the current priority. However for superusers it doesn't
really add anything either because thenice, and RLIMIT_NICE soft- and
hard limits can arbitrarily be raised.

So what is the idea behind RLIMIT_NICE ?

Thank you very much,

Jeroen


ps: please cc in reply.
--
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/

Extend procfs status for bonded links to show "backup" and "inactive" flags

[Jeroen van Bemmel]

Hi,

bond_info_show_slave in /drivers/net/bonding/bond_procfs.c currently 
does not show all the information that is relevant to determine whether 
a given slave is used for network traffic or not. In certain 
active-backup setups, this means one can not easily tell what's happening.


An example patch to fix this:

diff --git a/drivers/net/bonding/bond_procfs.c 
b/drivers/net/bonding/bond_procfs.c

index 3cea38d..48ac2a4 100644
--- a/drivers/net/bonding/bond_procfs.c
+++ b/drivers/net/bonding/bond_procfs.c
@@ -169,6 +169,8 @@ static void bond_info_show_slave(struct seq_file *seq,

seq_printf(seq, "\nSlave Interface: %s\n", slave->dev->name);
seq_printf(seq, "MII Status: %s\n", 
bond_slave_link_status(slave->link));

+   seq_printf(seq, "Bond Status: backup=%d inactive=%d\n",
+   slave->backup, slave->inactive );
if (slave->speed == SPEED_UNKNOWN)
seq_printf(seq, "Speed: %s\n", "Unknown");
else

Thanks,
Jeroen van Bemmel
--
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: forcedeth ethernet driver & Low power state

[Jeroen]

On Nov 25, 2007 7:36 PM, Robert Hancock <[EMAIL PROTECTED]> wrote:

> Are you sure forcedeth even supports that feature? I haven't seen any
> code for it, and certainly it should never be enabled by default..
>

The windows driver does. I have to disable it because otherwise I have lot's of
connection speed troubles. This is also what i see when I use a linux distro on
the server unfortunately I can't disable it.

-- 
Jeroen
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

forcedeth ethernet driver & Low power state

[Jeroen]

Hi,

I'm migrating my server from windows 2003 server to Ubuntu, but I am
stumbling over the "Low Power State Link Speed" option for my NIC
(forcedeth)

I need to disable this option in my windows driver otherwise the trough pout is
horrible because the link fluctuates constantly from 100/1000.

Anyway, my question is where and how can I turn off this feature for the
forcedeth driver? I've looked in the source and as far as I can tell there is no
bootoption for this. There are some references noted in the code, but AFAIK
no setting.

Any ideas? Thanks in advance!

-- 
Jeroen
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[Maybe FIXed] Re: Forcedeth issues, loss of connectivity. ASUS M2N32-SLI-D w/ AMD64

[Jeroen Roodhart]

Tobias Diedrich  tdiedrich.de> writes:
> 
> I've seen some issues with forcedeth too, but only with 2.6.20-rc5
> and -rc6 as far as I can say.  The box stopped receiving packets, but a
> "ifconfig eth0 promisc" 'fixed' it.

Sorry forgot to mention: I'm running FC6 with the 2.6.19-1.2895.fc6 x86_64 
kernel.
RPM says that it is based on:

* Wed Jan 10 2007 Dave Jones <[EMAIL PROTECTED]>
- 2.6.19.2

I tried to upgrade to the forcedeth driver that is supplied with 2.6.20-rc4-mm1
but that gave results that look like Tobias' description.

Then I downloaded the Nvidia provided version from:

http://download.nvidia.com/XFree86/nforce/1.21/NFORCE-Linux-x86-1.21.zip

The forcedeth driver from this file announces itself as:

Version 0.60-Driver Package V1.21.

After some minor patching I got this to work with the FC6 2.6.19-1.2895 kernel
and I just played 2 movies through the network whilst downloading a large number
of files onto the server. So far no network crashes. Maybe something for David
to try out as well?

Thanks for the information,

With kind regards,

Jeroen




-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: Forcedeth issues, loss of connectivity. ASUS M2N32-SLI-D w/ AMD64

[Jeroen Roodhart]

David Ford  blue-labs.org> writes:

> Any "me toos" out there, any suggestions?

Big "me too" here. I use this board to run "workstation and home samba services"
and it seems that under stress, the connection gets lost. Can't put my finger on
the when/what/why too.

I get the feeling though that when I use the ethernet-device locally _and_ it
gets used through samba (externally) the problem is more likely to occur.

If anyone gets this, please tell me. As David says: too nice a board to have
this problem :)

Thanx,

Jeroen Roodhart

-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

YABM (Yet another benchmark)

[Jeroen Vreeken]

Hi,
This benchmark was made in response to a recent post here on lkm were 
Linus indicated he would welcom pretty much any benchmark.
Since there are already several database benchmarks, 3d benchmarks I 
opted for a more down to earth approach.
As such I am pleased to announce the 'linux kernel hacker benchmark', a 
benchmark designed to simulate the activities of the average linux 
kernel hacker.
With this benchmark it should be possible to measure the performance off 
the kernel for its most important user group, the kernel hacker.

This workload turns out to be relativly simple to simulate as can be 
seen in the attached benchmark program 'lkh-bm.c'.
It is compiled with 'gcc -Wall lkh-bm.c -o lkh-bm'.

This test has been run on all 2.6 releases and several older kernels 
dating some years back. Unfortunatly 1.1 and lower kernels aren't able 
to complete the test.
The compiler used was gcc 3.2.3, the cpu a Celeron @ 2.4GHz.
I plan to run this test daily on all releases, bk, mm and ac snapshots 
and maybe more trees on kernel.org asuming nobody objects to me doing a 
recursive web-suck with wget.

At the end of this post you will find the already done benchmarks.
As Linus seems to dig pretty pictures a graph has been attached 
(lkh-bm.gif) with the same results.
Surprisingly the number seems to be constant during the last years. This 
could either indicate that the kernel hasn't regressed for years in this 
respect (which would mean somebody is doing a fine job indeed) or it 
could mean that the average kernel hacker simply doesn't do much usufull 
anyway

Regards,
Jeroen
Benchmark results:
1.1.0 0
1.1.20 0
1.1.40 0
1.1.60 0
1.2.0 603
2.0.0 604
2.0.10 605
2.0.20 600
2.0.30 601
2.2.0 602
2.2.10 603
2.2.20 604
2.4.0 605
2.4.10 600
2.4.20 601
2.6.0 602
2.6.1 603
2.6.2 604
2.6.3 605
2.6.4 600
2.6.5 601
2.6.6 602
2.6.7 603
2.6.8 604
2.6.9 605
2.6.10 600
2.6.11 601
<>#include 
#include 

#define MEASUREMENT_TIME	60
#define LINUS_CONSTANT		6

/*
 * my_integer_pi()
 *
 * This function calculates the value of PI, and returns
 * 3. It does so by adding "1" in a loop three times.
 */
int my_integer_pi(void)
{
	int i, pi;
	
	/*
	 * This is the main loop.
	 */
	pi = 0;
	for (i = 0; i < 3; i++)
		pi++;
	
	/* Ok, return it */
	return pi;
}

int main(int argc, char **argv)
{
	time_t timer, start, prev;
	int completed = 0;
	int calc;

	start = time(NULL);
	timer = start;
	prev = start;
	while ( timer - start <= MEASUREMENT_TIME ) {
		/* do some typical kernel hacker stuff... */
		calc = my_integer_pi();
		timer = time(NULL);
		if ((timer - prev) == LINUS_CONSTANT ) {
			completed++;
			prev = timer;
		}
	}
	printf("endless LKH loops per hour: %ld\n",
	completed * 3600 / MEASUREMENT_TIME + (time(NULL) % LINUS_CONSTANT));

	return 0;
}

IDE Raid supported with the HPT370?

[Jeroen Geusebroek]

Hi guys,

I have ordered a ABIT VP6 motherboard with the HPT370 controller
and would like to know if raid0 is supported with linux?

If not, will i be able to work without raid then? (maybe using
software raid)

Thanks,

Jeroen Geusebroek

P.s. Please CC me in your reply, since i'm not subscribed to this
list.

-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

2.2.17 & Eepro(10)

[Jeroen Geusebroek]

Hello,

I'm having troubles with the eepro driver included in kernel 2.2.17.
It stops sometimes with no apparent reason. The one thing i noticed
is that it seems to have a lot of carrier problems(998!)

This is part of the result from ifconfig:

eth1  Link encap:Ethernet  HWaddr 00:AA:00:A6:05:01  
  inet addr:24.132.xx.xxx  Bcast:24.132.xx.xxx  Mask:255.255.254.0
  UP BROADCAST RUNNING MULTICAST  MTU:1500  Metric:1
  RX packets:248714 errors:0 dropped:0 overruns:0 frame:0
  TX packets:64711 errors:1925 dropped:0 overruns:0 carrier:998
  collisions:832 txqueuelen:100 
  Interrupt:10 Base address:0x230

Needless to say i didn't have this problem with previous kernels
(including 2.2.16).

Is there something changed in the driver for 2.2.17?

Thanks for the help.

Regards,

Jeroen Geusebroek

P.s. please CC me if you reply since i'm not subscribed to the list. 
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
Please read the FAQ at http://www.tux.org/lkml/

[PATCH] vloopback 0.7

[Jeroen Vreeken]

Hello,

Attached to this message is a patch against 2.4.0-test8 with my
video4linux loopback driver.
I makes two video devices for each pipe created, one for input using
write and one for output using read or mmap.

Jeroen

diff -ruN linux/CREDITS linux-2.4.0-test8/CREDITS
--- linux/CREDITS   Fri Sep  8 21:38:00 2000
+++ linux-2.4.0-test8/CREDITS   Mon Sep 11 17:10:51 2000
@@ -2734,6 +2734,14 @@
 S: 1098 VA Amsterdam 
 S: The Netherlands
 
+N: Jeroen Vreeken
+E: [EMAIL PROTECTED]
+W: http://www.chello.nl/~j.vreeken/
+D: Video4linux loopback driver
+S: Maastrichterweg 63
+S: 5554 GG Valkenswaard
+S: The Netherlands
+
 N: Peter Shaobo Wang
 E: [EMAIL PROTECTED]
 W: http://www.mmdcorp.com/pw/linux
diff -ruN linux/drivers/media/video/Config.in 
linux-2.4.0-test8/drivers/media/video/Config.in
--- linux/drivers/media/video/Config.in Wed Aug 23 23:59:55 2000
+++ linux-2.4.0-test8/drivers/media/video/Config.in Mon Sep 11 17:10:51 2000
@@ -38,6 +38,7 @@
fi
dep_tristate '  Stradis 4:2:2 MPEG-2 video driver  (EXPERIMENTAL)' 
CONFIG_VIDEO_STRADIS $CONFIG_VIDEO_DEV $CONFIG_PCI
 fi
+dep_tristate '  Video For Linux loopback' CONFIG_VIDEO_VLOOPBACK $CONFIG_VIDEO_DEV
 dep_tristate '  Zoran ZR36057/36060 Video For Linux' CONFIG_VIDEO_ZORAN 
$CONFIG_VIDEO_DEV $CONFIG_PCI $CONFIG_I2C
 dep_tristate 'Include support for Iomega Buz' CONFIG_VIDEO_BUZ $CONFIG_VIDEO_ZORAN
 dep_tristate '  Zoran ZR36120/36125 Video For Linux' CONFIG_VIDEO_ZR36120 
$CONFIG_VIDEO_DEV $CONFIG_PCI $CONFIG_I2C
diff -ruN linux/drivers/media/video/Makefile 
linux-2.4.0-test8/drivers/media/video/Makefile
--- linux/drivers/media/video/Makefile  Tue Aug 22 20:29:02 2000
+++ linux-2.4.0-test8/drivers/media/video/Makefile  Mon Sep 11 17:10:51 2000
@@ -57,6 +57,7 @@
 obj-$(CONFIG_VIDEO_CPIA) += cpia.o
 obj-$(CONFIG_VIDEO_CPIA_PP) += cpia_pp.o
 obj-$(CONFIG_VIDEO_CPIA_USB) += cpia_usb.o
+obj-$(CONFIG_VIDEO_VLOOPBACK) += vloopback.o
 obj-$(CONFIG_TUNER_3036) += tuner-3036.o
 
 # Extract lists of the multi-part drivers.
diff -ruN linux/drivers/media/video/vloopback.c 
linux-2.4.0-test8/drivers/media/video/vloopback.c
--- linux/drivers/media/video/vloopback.c   Thu Jan  1 01:00:00 1970
+++ linux-2.4.0-test8/drivers/media/video/vloopback.c   Mon Sep 11 17:11:12 2000
@@ -0,0 +1,816 @@
+/*
+ * vloopback.c
+ *
+ * Copyright Jeroen Vreeken ([EMAIL PROTECTED]), 2000
+ *
+ * Published under the GNU Public License.
+ *
+ *
+ * UPDATED:Jeroen Vreeken.
+ * Added locks for smp machines. UNTESTED!
+ * Made the driver much more cpu friendly by using
+ * a wait queue.
+ * Went from vmalloc to rvmalloc (yes, I stole the code
+ * like everybody else) and implemented mmap.
+ * Implemented VIDIOCGUNIT and removed size/palette checks
+ * in VIDIOCSYNC.
+ * Cleaned up a lot of code.
+ * Changed locks to semaphores.
+ * Disabled changing size while somebody is using mmap
+ * Changed mapped check to open check, also don't allow
+ * a open for write while somebody is reading.
+ * Added /proc support
+ */
+#define VLNAME "vloopback: "
+#define VLVER "0.7"
+
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+
+struct vloopback_device {
+   struct video_device viddev;
+   int pipenr;
+   int in;
+};
+
+struct vloopback_pipe {
+   struct vloopback_device *vloopin;
+   struct vloopback_device *vloopout;
+   char *buffer;
+   unsigned long buflength;
+   unsigned int width, height;
+   unsigned int palette;
+   unsigned long frameswrite;
+   unsigned long framesread;
+   unsigned long framesdumped;
+   unsigned int wopen;
+   unsigned int ropen;
+   struct proc_dir_entry *proc_entry;
+   struct semaphore lock;
+   wait_queue_head_t wait;
+};
+
+#define MAX_PIPES 16
+static struct vloopback_pipe *loops[MAX_PIPES];
+static int nr_o_pipes=0;
+static int pipes=-1;
+
+
+/**
+ *
+ * Memory management
+ *
+ * This is a shameless copy from the USB-cpia driver (linux kernel
+ * version 2.3.29 or so, I have no idea what this code actually does ;).
+ * Actually it seems to be a copy of a shameless copy of the bttv-driver.
+ * Or that is a copy of a shameless copy of ... (To the powers: is there
+ * no generic kernel-function to do this sort of stuff?)
+ *
+ * Yes, it was a shameless copy from the bttv-driver. IIRC, Alan says
+ * there will be one, but apparentely not yet -jerdfelt
+ *
+ * So I copied it again for the OV511 driver -claudio
+ *
+ * And it gets copied and copied and copied. -Jeroen
+ *
+