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, 2 Jun 2021 04:39:20 +0200
From:   Andrew Lunn <andrew@lunn.ch>
To:     Joakim Zhang <qiangqing.zhang@nxp.com>
Cc:     davem@davemloft.net, kuba@kernel.org, robh+dt@kernel.org,
        hkallweit1@gmail.com, linux@armlinux.org.uk, f.fainelli@gmail.com,
        linux-imx@nxp.com, netdev@vger.kernel.org,
        devicetree@vger.kernel.org, linux-kernel@vger.kernel.org
Subject: Re: [PATCH net-next 1/4] dt-bindings: net: add dt binding for
 realtek rtl82xx phy
Message-ID: <YLbvWKIE3FOhdzsl@lunn.ch>
References: <20210601090408.22025-1-qiangqing.zhang@nxp.com>
 <20210601090408.22025-2-qiangqing.zhang@nxp.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <20210601090408.22025-2-qiangqing.zhang@nxp.com>
Precedence: bulk

> +properties:
> +  rtl821x,clkout-disable:
> +    description: Disable CLKOUT clock.
> +    type: boolean
> +
> +  rtl821x,aldps-disable:
> +    description: Disable ALDPS mode.
> +    type: boolean

I think most of the problems are the ambiguity in the binding.

If rtl821x,clkout-disable is not present, should it enable the CLKOUT?
That needs clear define here.

Do we actually want a tristate here?

                rtl821x,clkout = <true>;

means ensure the clock is outputting.

                rtl821x,clkout = <false>;

means ensure the clock is not outputting.

And if the property is not in DT at all, leave the hardware alone, at
either its default value, or whatever came before has set it to?

    Andrew