On Tue, Feb 11, 2014 at 04:29:22PM +0100, Stefan Sørensen wrote:
> diff --git a/Documentation/devicetree/bindings/net/dp83640.txt b/Documentation/devicetree/bindings/net/dp83640.txt
> new file mode 100644
> index 0000000..b9a57c0
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/net/dp83640.txt
> @@ -0,0 +1,29 @@
> +Required properties for the National DP83640 ethernet phy:
> +
> +- compatible : Must contain "national,dp83640"
> +
> +Optional properties:
> +
> +- dp83640,slave: If present, this phy will be slave to another dp83640
> + on the same mdio bus.
Wouldn't it be more natural to have one "dp83640,master" property
rather than multiple slave properties?
> @@ -949,6 +940,95 @@ static void dp83640_clock_put(struct dp83640_clock *clock)
> mutex_unlock(&clock->clock_lock);
> }
>
> +#ifdef CONFIG_OF
> +static int dp83640_probe_dt(struct device_node *node,
> + struct dp83640_private *dp83640)
> +{
> + struct dp83640_clock *clock = dp83640->clock;
> + struct property *prop;
> + int err, proplen;
> +
> + dp83640->slave = of_property_read_bool(node, "dp83640,slave");
> + if (!dp83640->slave && clock->chosen) {
> + pr_err("dp83640,slave must be set if more than one device on the same bus");
Most of these pr_err lines are a bit _way_ too long for coding style.
> + return -EINVAL;
> + }
> +
> + prop = of_find_property(node, "dp83640,perout-pins", &proplen);
> + if (prop) {
> + if (dp83640->slave) {
> + pr_err("dp83640,perout-pins property can not be set together with dp83640,slave");
(Here especially and in the code that followed.)
Overall the series is looking better. I will try to test the non-DT
case later on this week.
Thanks,
Richard
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
