Re: [PATCH 1/3] dt-bindings: can: xilinx_can: Add ECC property ‘xlnx,has-ecc’

From: Michal Simek
Date: Mon Jun 19 2023 - 02:39:13 EST




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 <srinivas.goud@xxxxxxx>

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