ECC feature added to Tx and Rx FIFO’s for Xilinx CAN Controller.
Part of this feature configuration and counter registers
added in IP for 1bit/2bit ECC errors.
Please find more details in PG096 v5.1 document.
xlnx,has-ecc is optional property and added to Xilinx CAN
Controller node if ECC block enabled in the HW.
Signed-off-by: Srinivas Goud <[email protected]>
---
Documentation/devicetree/bindings/net/can/xilinx,can.yaml | 5 +++++
1 file changed, 5 insertions(+)
diff --git a/Documentation/devicetree/bindings/net/can/xilinx,can.yaml b/Documentation/devicetree/bindings/net/can/xilinx,can.yaml
index 897d2cb..13503ae 100644
--- a/Documentation/devicetree/bindings/net/can/xilinx,can.yaml
+++ b/Documentation/devicetree/bindings/net/can/xilinx,can.yaml
@@ -46,6 +46,10 @@ properties:
$ref: /schemas/types.yaml#/definitions/uint32
description: CAN Tx mailbox buffer count (CAN FD)
+ xlnx,has-ecc:
+ $ref: /schemas/types.yaml#/definitions/flag
+ description: CAN Tx and Rx fifo ECC enable flag (AXI CAN)
+
required:
- compatible
- reg
@@ -134,6 +138,7 @@ examples:
interrupts = <GIC_SPI 59 IRQ_TYPE_EDGE_RISING>;
tx-fifo-depth = <0x40>;
rx-fifo-depth = <0x40>;
+ xlnx,has-ecc;
};
- |
--
2.1.1
On 12.06.2023 17:12:55, Srinivas Goud wrote:
> ECC feature added to Tx and Rx FIFO’s for Xilinx CAN Controller.
> Part of this feature configuration and counter registers
> added in IP for 1bit/2bit ECC errors.
> Please find more details in PG096 v5.1 document.
>
> xlnx,has-ecc is optional property and added to Xilinx CAN
> Controller node if ECC block enabled in the HW.
>
> Signed-off-by: Srinivas Goud <[email protected]>
Is there a way to introspect the IP core to check if this feature is
compiled in?
Marc
--
Pengutronix e.K. | Marc Kleine-Budde |
Embedded Linux | https://www.pengutronix.de |
Vertretung Nürnberg | Phone: +49-5121-206917-129 |
Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917-9 |
On 12/06/2023 13:42, Srinivas Goud wrote:
> ECC feature added to Tx and Rx FIFO’s for Xilinx CAN Controller.
> Part of this feature configuration and counter registers
> added in IP for 1bit/2bit ECC errors.
> Please find more details in PG096 v5.1 document.
>
> xlnx,has-ecc is optional property and added to Xilinx CAN
> Controller node if ECC block enabled in the HW.
Please use scripts/get_maintainers.pl to get a list of necessary people
and lists to CC. It might happen, that command when run on an older
kernel, gives you outdated entries. Therefore please be sure you base
your patches on recent Linux kernel.
You missed at least DT list (maybe more), so this won't be tested by our
tools. Performing review on untested code might be a waste of time, thus
I will skip this patch entirely till you follow the process allowing the
patch to be tested.
Please kindly resend and include all necessary To/Cc entries.
Best regards,
Krzysztof
Hi,
>-----Original Message-----
>From: Marc Kleine-Budde <[email protected]>
>Sent: Tuesday, June 13, 2023 1:23 PM
>To: Goud, Srinivas <[email protected]>
>Cc: [email protected]; [email protected]; [email protected];
>[email protected]; [email protected]; [email protected]; git (AMD-
>Xilinx) <[email protected]>; [email protected]; [email protected];
>[email protected]; [email protected]
>Subject: Re: [PATCH 1/3] dt-bindings: can: xilinx_can: Add ECC property
>‘xlnx,has-ecc’
>
>On 12.06.2023 17:12:55, Srinivas Goud wrote:
>> ECC feature added to Tx and Rx FIFO’s for Xilinx CAN Controller.
>> Part of this feature configuration and counter registers added in IP
>> for 1bit/2bit ECC errors.
>> Please find more details in PG096 v5.1 document.
>>
>> xlnx,has-ecc is optional property and added to Xilinx CAN Controller
>> node if ECC block enabled in the HW.
>>
>> Signed-off-by: Srinivas Goud <[email protected]>
>
>Is there a way to introspect the IP core to check if this feature is compiled in?
There is no way(IP registers) to indicate whether ECC feature is enabled or not.
>
>Marc
>
>--
>Pengutronix e.K. | Marc Kleine-Budde |
>Embedded Linux | https://www.pengutronix.de |
>Vertretung Nürnberg | Phone: +49-5121-206917-129 |
>Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917-9 |
Thanks,
Srinivas
On 14/06/2023 12:22, Goud, Srinivas wrote:
> Hi,
>
>> -----Original Message-----
>> From: Marc Kleine-Budde <[email protected]>
>> Sent: Tuesday, June 13, 2023 1:23 PM
>> To: Goud, Srinivas <[email protected]>
>> Cc: [email protected]; [email protected]; [email protected];
>> [email protected]; [email protected]; [email protected]; git (AMD-
>> Xilinx) <[email protected]>; [email protected]; [email protected];
>> [email protected]; [email protected]
>> Subject: Re: [PATCH 1/3] dt-bindings: can: xilinx_can: Add ECC property
>> ‘xlnx,has-ecc’
>>
>> On 12.06.2023 17:12:55, Srinivas Goud wrote:
>>> ECC feature added to Tx and Rx FIFO’s for Xilinx CAN Controller.
>>> Part of this feature configuration and counter registers added in IP
>>> for 1bit/2bit ECC errors.
>>> Please find more details in PG096 v5.1 document.
>>>
>>> xlnx,has-ecc is optional property and added to Xilinx CAN Controller
>>> node if ECC block enabled in the HW.
>>>
>>> Signed-off-by: Srinivas Goud <[email protected]>
>>
>> Is there a way to introspect the IP core to check if this feature is compiled in?
> There is no way(IP registers) to indicate whether ECC feature is enabled or not.
Isn't this then deductible from compatible? Your binding claims it is
only for AXI CAN, so xlnx,axi-can-1.00.a, even though you did not
restrict it in the binding.
Best regards,
Krzysztof
Hi
>-----Original Message-----
>From: Krzysztof Kozlowski <[email protected]>
>Sent: Wednesday, June 14, 2023 4:41 PM
>To: Goud, Srinivas <[email protected]>; Marc Kleine-Budde
><[email protected]>
>Cc: [email protected]; [email protected]; [email protected];
>[email protected]; [email protected]; [email protected]; git (AMD-
>Xilinx) <[email protected]>; [email protected]; [email protected];
>[email protected]; [email protected]
>Subject: Re: [PATCH 1/3] dt-bindings: can: xilinx_can: Add ECC property
>‘xlnx,has-ecc’
>
>On 14/06/2023 12:22, Goud, Srinivas wrote:
>> Hi,
>>
>>> -----Original Message-----
>>> From: Marc Kleine-Budde <[email protected]>
>>> Sent: Tuesday, June 13, 2023 1:23 PM
>>> To: Goud, Srinivas <[email protected]>
>>> Cc: [email protected]; [email protected]; [email protected];
>>> [email protected]; [email protected]; [email protected]; git (AMD-
>>> Xilinx) <[email protected]>; [email protected];
>>> [email protected]; [email protected];
>>> [email protected]
>>> Subject: Re: [PATCH 1/3] dt-bindings: can: xilinx_can: Add ECC
>>> property ‘xlnx,has-ecc’
>>>
>>> On 12.06.2023 17:12:55, Srinivas Goud wrote:
>>>> ECC feature added to Tx and Rx FIFO’s for Xilinx CAN Controller.
>>>> Part of this feature configuration and counter registers added in IP
>>>> for 1bit/2bit ECC errors.
>>>> Please find more details in PG096 v5.1 document.
>>>>
>>>> xlnx,has-ecc is optional property and added to Xilinx CAN Controller
>>>> node if ECC block enabled in the HW.
>>>>
>>>> Signed-off-by: Srinivas Goud <[email protected]>
>>>
>>> Is there a way to introspect the IP core to check if this feature is compiled in?
>> There is no way(IP registers) to indicate whether ECC feature is enabled or
>not.
>
>Isn't this then deductible from compatible? Your binding claims it is only for
>AXI CAN, so xlnx,axi-can-1.00.a, even though you did not restrict it in the
>binding.
Agree it is only for AXI CAN(xlnx,axi-can-1.00.a) but ECC feature is
configurable option to the user.
ECC is added as optional configuration(enable/disable) feature
to the existing AXI CAN.
Thanks,
Srinivas
On 16/06/2023 12:13, Goud, Srinivas wrote:
>>>>> xlnx,has-ecc is optional property and added to Xilinx CAN Controller
>>>>> node if ECC block enabled in the HW.
>>>>>
>>>>> Signed-off-by: Srinivas Goud <[email protected]>
>>>>
>>>> Is there a way to introspect the IP core to check if this feature is compiled in?
>>> There is no way(IP registers) to indicate whether ECC feature is enabled or
>> not.
>>
>> Isn't this then deductible from compatible? Your binding claims it is only for
>> AXI CAN, so xlnx,axi-can-1.00.a, even though you did not restrict it in the
>> binding.
> Agree it is only for AXI CAN(xlnx,axi-can-1.00.a) but ECC feature is
> configurable option to the user.
> ECC is added as optional configuration(enable/disable) feature
> to the existing AXI CAN.
Why boards would like not to have ECC? I understand that someone told
you "make it configurable in DTS", but that's not really a reason for
us. Why this is suitable for DTS?
Best regards,
Krzysztof
On 6/16/23 12:38, Krzysztof Kozlowski wrote:
> On 16/06/2023 12:13, Goud, Srinivas wrote:
>>>>>> xlnx,has-ecc is optional property and added to Xilinx CAN Controller
>>>>>> node if ECC block enabled in the HW.
>>>>>>
>>>>>> Signed-off-by: Srinivas Goud <[email protected]>
>>>>>
>>>>> Is there a way to introspect the IP core to check if this feature is compiled in?
>>>> There is no way(IP registers) to indicate whether ECC feature is enabled or
>>> not.
>>>
>>> Isn't this then deductible from compatible? Your binding claims it is only for
>>> AXI CAN, so xlnx,axi-can-1.00.a, even though you did not restrict it in the
>>> binding.
>> Agree it is only for AXI CAN(xlnx,axi-can-1.00.a) but ECC feature is
>> configurable option to the user.
>> ECC is added as optional configuration(enable/disable) feature
>> to the existing AXI CAN.
>
> Why boards would like not to have ECC? I understand that someone told
> you "make it configurable in DTS", but that's not really a reason for
> us. Why this is suitable for DTS?
Let me jump to this. This is core for programmable logic where HW designers of
this IP added couple of feature which can be enabled or disable based on
customer need. It means it is not SW option if ECC is enable but it is HW choice
if ECC is present in the HW or not.
Selection if ECC should be used is up to every customer to decide.
We are not able to get information why customers choosing ECC enabled/disabled
but I can imagine that with ECC disable less fpga resources are used.
Thanks,
Michal
On 16/06/2023 12:44, Michal Simek wrote:
>
>
> On 6/16/23 12:38, Krzysztof Kozlowski wrote:
>> On 16/06/2023 12:13, Goud, Srinivas wrote:
>>>>>>> xlnx,has-ecc is optional property and added to Xilinx CAN Controller
>>>>>>> node if ECC block enabled in the HW.
>>>>>>>
>>>>>>> Signed-off-by: Srinivas Goud <[email protected]>
>>>>>>
>>>>>> Is there a way to introspect the IP core to check if this feature is compiled in?
>>>>> There is no way(IP registers) to indicate whether ECC feature is enabled or
>>>> not.
>>>>
>>>> Isn't this then deductible from compatible? Your binding claims it is only for
>>>> AXI CAN, so xlnx,axi-can-1.00.a, even though you did not restrict it in the
>>>> binding.
>>> Agree it is only for AXI CAN(xlnx,axi-can-1.00.a) but ECC feature is
>>> configurable option to the user.
>>> ECC is added as optional configuration(enable/disable) feature
>>> to the existing AXI CAN.
>>
>> Why boards would like not to have ECC? I understand that someone told
>> you "make it configurable in DTS", but that's not really a reason for
>> us. Why this is suitable for DTS?
>
> Let me jump to this. This is core for programmable logic where HW designers of
> this IP added couple of feature which can be enabled or disable based on
> customer need. It means it is not SW option if ECC is enable but it is HW choice
> if ECC is present in the HW or not.
> Selection if ECC should be used is up to every customer to decide.
> We are not able to get information why customers choosing ECC enabled/disabled
> but I can imagine that with ECC disable less fpga resources are used.
Thanks for the explanation. Apologies for being picky, but you are in
minority when adding such properties with true hardware meaning. Most of
the submissions of such properties add them to control the bits in register.
Best regards,
Krzysztof
On 14/06/2023 13:11, Krzysztof Kozlowski wrote:
> On 14/06/2023 12:22, Goud, Srinivas wrote:
>> Hi,
>>
>>> -----Original Message-----
>>> From: Marc Kleine-Budde <[email protected]>
>>> Sent: Tuesday, June 13, 2023 1:23 PM
>>> To: Goud, Srinivas <[email protected]>
>>> Cc: [email protected]; [email protected]; [email protected];
>>> [email protected]; [email protected]; [email protected]; git (AMD-
>>> Xilinx) <[email protected]>; [email protected]; [email protected];
>>> [email protected]; [email protected]
>>> Subject: Re: [PATCH 1/3] dt-bindings: can: xilinx_can: Add ECC property
>>> ‘xlnx,has-ecc’
>>>
>>> On 12.06.2023 17:12:55, Srinivas Goud wrote:
>>>> ECC feature added to Tx and Rx FIFO’s for Xilinx CAN Controller.
>>>> Part of this feature configuration and counter registers added in IP
>>>> for 1bit/2bit ECC errors.
>>>> Please find more details in PG096 v5.1 document.
>>>>
>>>> xlnx,has-ecc is optional property and added to Xilinx CAN Controller
>>>> node if ECC block enabled in the HW.
>>>>
>>>> Signed-off-by: Srinivas Goud <[email protected]>
>>>
>>> Is there a way to introspect the IP core to check if this feature is compiled in?
>> There is no way(IP registers) to indicate whether ECC feature is enabled or not.
>
> Isn't this then deductible from compatible? Your binding claims it is
> only for AXI CAN, so xlnx,axi-can-1.00.a, even though you did not
> restrict it in the binding.
BTW, this is not an ACK, because this was not tested by automation. I
don't understand why get_maintainers.pl are so tricky to use, but
nevertheless I require resend to satisfy automation.
Best regards,
Krzysztof
On 6/17/23 09:31, Krzysztof Kozlowski wrote:
> On 16/06/2023 12:44, Michal Simek wrote:
>>
>>
>> On 6/16/23 12:38, Krzysztof Kozlowski wrote:
>>> On 16/06/2023 12:13, Goud, Srinivas wrote:
>>>>>>>> xlnx,has-ecc is optional property and added to Xilinx CAN Controller
>>>>>>>> node if ECC block enabled in the HW.
>>>>>>>>
>>>>>>>> Signed-off-by: Srinivas Goud <[email protected]>
>>>>>>>
>>>>>>> Is there a way to introspect the IP core to check if this feature is compiled in?
>>>>>> There is no way(IP registers) to indicate whether ECC feature is enabled or
>>>>> not.
>>>>>
>>>>> Isn't this then deductible from compatible? Your binding claims it is only for
>>>>> AXI CAN, so xlnx,axi-can-1.00.a, even though you did not restrict it in the
>>>>> binding.
>>>> Agree it is only for AXI CAN(xlnx,axi-can-1.00.a) but ECC feature is
>>>> configurable option to the user.
>>>> ECC is added as optional configuration(enable/disable) feature
>>>> to the existing AXI CAN.
>>>
>>> Why boards would like not to have ECC? I understand that someone told
>>> you "make it configurable in DTS", but that's not really a reason for
>>> us. Why this is suitable for DTS?
>>
>> Let me jump to this. This is core for programmable logic where HW designers of
>> this IP added couple of feature which can be enabled or disable based on
>> customer need. It means it is not SW option if ECC is enable but it is HW choice
>> if ECC is present in the HW or not.
>> Selection if ECC should be used is up to every customer to decide.
>> We are not able to get information why customers choosing ECC enabled/disabled
>> but I can imagine that with ECC disable less fpga resources are used.
>
> Thanks for the explanation. Apologies for being picky, but you are in
> minority when adding such properties with true hardware meaning. Most of
> the submissions of such properties add them to control the bits in register.
No issue at all. We are talking to HW designers to change their mindset and help
us with automatic detection of these features but truth is that every such a
feature means fpga resources that's why they are trying to avoid it to save them
and help customers to fit as much as possible to their fpgas. Because bigger
fpga is more expensive and also consumes more power.
Thanks,
Michal