Enhance if_cgem (add SGMII connection type, rewrite errata logic, fix typo)

[bsd_dino.sk]

Summary

Cadence GEM core is used in various ARM, ARM64 and RISCV systems. There are some errata/quirks in different implementations, presented patch changes logic to enable them. As PolarFire SoC needs SGMII to connect PHY, 'phy-mode' property of device tree node for ethernet is checked and acted on appropriately. Also, fix a typo in if_cgem_hw.h file.

Test Plan

Build a kernel/module and test on available devices (Zynq based Zybo Z7 and Cora Z7 boards, PolarFire SoC based PF-RCHD SoM+carrier board from Conclusive Engineering) - it should be no change for all but PolarFire SoC based boards/devices.

[mhorne]

Hi, thanks for the submission.

The quirk handling changes seem like a good improvement to me, but maybe @skibo has more of an opinion. They should be committed separately from the phy_contype + typo changes.

sys/dev/cadence/if_cgem.c

113

What device tree provides this compatible? I do not see it in the upstreamed bits for the PolarFire/MPFS Icicle Kit.

1748

FYI, it is preferred (but not enforced) style within FreeBSD to be explicit like this for flag checks. You can see it elsewhere in this file.

1752–1753

Could this...

1766–1769

...be moved down here? So that it is not in the middle of checking the quirks.

[skibo]

sys/dev/cadence/if_cgem.c

118

The problem with this is that HWQUIRK_NONE is zero which also means no match in the probe routine. Any board that uses the last three generic compatible strings won't match. How about HWQUIRK_NONE be 1 and move the other bits left.

[bsd_dino.sk]

In D34764#787943, @mhorne wrote:

The quirk handling changes ... should be committed separately from the phy_contype + typo changes.

No problem with this, I just think it could speed up the process a bit to discuss those together...

sys/dev/cadence/if_cgem.c

113

It is used in device tree presented from U-Boot as used in PF-RCHD SoM V1.0 - this is not published anywhere I think. I saw some compatible changes in MicroChip device trees, anyway, so it may change in future, too, but now it is this way. It would be great if someone with Icicle Kit could check, but I know no one who has it.

1748

OK. Wait a moment, please, for new revision...

1766–1769

Definitelly, yes. In some older version it was not that visible. Here it is clearly better the way you show.

[bsd_dino.sk]

Inline comments made by mhorne addressed...

[bsd_dino.sk]

sys/dev/cadence/if_cgem.c

118

Ah, that's an oversight from me... I have another idea I need to check first. Wait a moment, please.

[bsd_dino.sk]

Fix probe issue pointed to by skibo

[bsd_dino.sk]

sys/dev/cadence/if_cgem.c

1726

This change should solve the probe issue. While more commonly comparison ocd_data == 0 is used, ocd_str == NULL is logically equivalent and used couple of times as well.

[mhorne]

In D34764#787954, @bsd_dino.sk wrote:

In D34764#787943, @mhorne wrote:

The quirk handling changes ... should be committed separately from the phy_contype + typo changes.

No problem with this, I just think it could speed up the process a bit to discuss those together...

Indeed, and it did. As with your other patches I can handle the small cleanups like this before commit.