Re: [PATCH v3 7/8] riscv: dts: eswin: add I2C controller support

Hi Sashiko,

> Thank you for your contribution! Sashiko AI review found 1 potential issu=
e(s)
> to consider:
> - [High] Incorrect pinctrl assignment for AON I2C controllers, mapping th=
em to
> LSP I2C pins instead of AON I2C pins.
> --
>=20
> --- Patch [7]: [PATCH v3 7/8] riscv: dts: eswin: add I2C controller suppo=
rt ---
> commit dd055880a3b8b5096248d3b1a4bdcec3e5acbe01
> Author: Pritesh Patel <[email protected]>
>=20
> riscv: dts: eswin: add I2C controller support
>=20
> This commit adds I2C nodes for the EIC7700 SoC and enables corresponding
> slave devices for the HiFive Premier P550 board.
>=20
> > diff --git a/arch/riscv/boot/dts/eswin/eic7700-hifive-premier-p550.dts
> > b/arch/riscv/boot/dts/eswin/eic7700-hifive-premier-p550.dts
> > index 3101fd90d1714..50c15e08a57f8 100644
> > --- a/arch/riscv/boot/dts/eswin/eic7700-hifive-premier-p550.dts
> > +++ b/arch/riscv/boot/dts/eswin/eic7700-hifive-premier-p550.dts
>=20
> [ ... ]
>=20
> > @@ -130,6 +130,62 @@ &gpio111_pins {
> >       input-disable;
> >  };
> >
> > +&aon_i2c0 {
> > +     status =3D "okay";
> > +     pinctrl-names =3D "default";
> > +     pinctrl-0 =3D <&i2c0_pins>;
>=20
> [Severity: High]
> Should this use the AON pins (&i2c10_pins) instead of the LSP pins?
>=20
> Assigning &i2c0_pins here configures the pads for the Low Speed Periphera=
l
> i2c0 controller, leaving the AON I2C bus disconnected from the external p=
ads
> and preventing communication with the EEPROM.

Yes. We will correct this in next version for both I2C.

Regards,
Pinkesh

>=20
> > +
> > +     eeprom@50 {
> > +             compatible =3D "atmel,24c02";
> > +             reg =3D <0x50>;
> > +     };
> > +};
> > +
> > +&aon_i2c1 {
> > +     status =3D "okay";
> > +     pinctrl-names =3D "default";
> > +     pinctrl-0 =3D <&i2c1_pins>;
>=20
> [Severity: High]
> Similarly, should this use &i2c11_pins for the aon_i2c1 controller?
>=20
> Routing the AON controller to the LSP i2c1 pins will cause the ADC and po=
wer
> sensor to fail on boot due to being physically disconnected.
>=20
> > +
> > +     adc@10 {
>=20
> [ ... ]
>=20
> --
> Sashiko AI review *
> https://nam04.safelinks.protection.outlook.com/?url=3Dhttps%3A%2F%2Fsashi=
k
> o.dev%2F%23%2Fpatchset%2F20260804104431.1391839-1-
> pinkesh.vaghela%40einfochips.com%3Fpart%3D7&data=3D05%7C02%7Cpinkesh
> .vaghela%40einfochips.com%7Cd8bc429a6c0946a6b9da08def21ab9f5%7C0b
> eb0c359cbb4feb99e5589e415c7944%7C1%7C0%7C639214393771766036%7C
> Unknown%7CTWFpbGZsb3d8eyJFbXB0eU1hcGkiOnRydWUsIlYiOiIwLjAuMDA
> wMCIsIlAiOiJXaW4zMiIsIkFOIjoiTWFpbCIsIldUIjoyfQ%3D%3D%7C0%7C%7C%
> 7C&sdata=3D0EXL5kcwVb9qtE8hUzTyjhOYtlomJ2zP5N3MVrMBBmM%3D&reserv
> ed=3D0