Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
Date:   Wed, 10 Mar 2021 09:39:57 +0100
From:   Maxime Ripard <maxime@cerno.tech>
To:     Evgeny Boger <boger@wirenboard.com>
Cc:     Chen-Yu Tsai <wens@csie.org>, linux-arm-kernel@lists.infradead.org,
        devicetree@vger.kernel.org, linux-kernel@vger.kernel.org,
        Rob Herring <robh+dt@kernel.org>
Subject: Re: [PATCH v2 1/2] net: allwinner: reset control support
Message-ID: <20210310083957.utmn7twsm2qgksfp@gilmour>
References: <20210309012116.2944-1-boger@wirenboard.com>
 <20210309012116.2944-2-boger@wirenboard.com>
MIME-Version: 1.0
Content-Type: multipart/signed; micalg=pgp-sha256;
        protocol="application/pgp-signature"; boundary="uizhoovv4z7qhw72"
Content-Disposition: inline
In-Reply-To: <20210309012116.2944-2-boger@wirenboard.com>
Precedence: bulk


--uizhoovv4z7qhw72
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
Content-Transfer-Encoding: quoted-printable

Hi,

On Tue, Mar 09, 2021 at 04:21:15AM +0300, Evgeny Boger wrote:
> R40 (aka V40/A40i/T3) and A10/A20 share the same EMAC IP.
> However, on R40 the EMAC is gated by default.
>=20
> Signed-off-by: Evgeny Boger <boger@wirenboard.com>
> ---
>  .../net/allwinner,sun4i-a10-emac.yaml         | 11 +++-
>  drivers/net/ethernet/allwinner/sun4i-emac.c   | 65 +++++++++++++++++--
>  2 files changed, 70 insertions(+), 6 deletions(-)
>=20
> diff --git a/Documentation/devicetree/bindings/net/allwinner,sun4i-a10-em=
ac.yaml b/Documentation/devicetree/bindings/net/allwinner,sun4i-a10-emac.ya=
ml
> index 8d8560a67abf..27f99372d153 100644
> --- a/Documentation/devicetree/bindings/net/allwinner,sun4i-a10-emac.yaml
> +++ b/Documentation/devicetree/bindings/net/allwinner,sun4i-a10-emac.yaml
> @@ -15,7 +15,12 @@ maintainers:
> =20
>  properties:
>    compatible:
> -    const: allwinner,sun4i-a10-emac
> +    oneOf:
> +      - const: allwinner,sun4i-a10-emac
> +      - const: allwinner,sun4i-r40-emac
> +      - items:
> +          - const: allwinner,sun4i-r40-emac
> +          - const: allwinner,sun4i-a10-emac

There's no need to handle the fallback case, it should have either one
of the two, not both.

The good news is that it simplifies the binding here too, since you can
use an enum

The DT binding modifications are usually in a separate patch too

> =20
>    reg:
>      maxItems: 1
> @@ -30,6 +35,9 @@ properties:
>      description: Phandle to the device SRAM
>      $ref: /schemas/types.yaml#/definitions/phandle-array
> =20
> +  resets:
> +    maxItems: 1
> +

You should make resets required for the R40 compatible too through an if
clause.

It looks good otherwise, thanks!
Maxime

--uizhoovv4z7qhw72
Content-Type: application/pgp-signature; name="signature.asc"

-----BEGIN PGP SIGNATURE-----

iHUEABYIAB0WIQRcEzekXsqa64kGDp7j7w1vZxhRxQUCYEiF2AAKCRDj7w1vZxhR
xWf+AQCWKzhut6ZPGHD5e8P7IIIhhkd7RuiPg8Dkq0nZaHFZqwEAwH3hAqMwcQ8U
GKPOy+EvneB2MZR+8Ry5N/mB9HicQwg=
=t6wL
-----END PGP SIGNATURE-----

--uizhoovv4z7qhw72--