On Sunday 15 February 2015 13:09:10 Eliad Peller wrote:
s
> +
> +This node provides properties for controlling the wilink wireless device. The
> +node is expected to be specified as a child node to the SDIO controller that
> +connects the device to the system.
> +
> +Required properties:
> +
> + - compatible : Should be "ti,wlcore".
I think you should use the specific model number here. If I understand
correctly, wlcore is the name of the driver that is used for multiple
device implementation.
> + - interrupt-parent : the phandle for the interrupt controller to which the
> + device interrupts are connected.
interrupt-parent should not be required
> +&mmc3 {
> + status = "okay";
> + vmmc-supply = <&wlan_en_reg>;
> + bus-width = <4>;
> + cap-power-off-card;
> + keep-power-in-suspend;
> +
> + #address-cells = <1>;
> + #size-cells = <0>;
> + wlcore: wlcore@0 {
> + compatible = "ti,wlcore";
> + reg = <2>;
> + interrupt-parent = <&gpio0>;
> + interrupts = <19 IRQ_TYPE_NONE>;
> + };
> +};
It could make sense to specify a few extra properties here:
- The platform data lists two clocks. How about adding them
here as optional clocks so we don't need to change the binding
again.
> diff --git a/drivers/net/wireless/ti/wlcore/sdio.c b/drivers/net/wireless/ti/wlcore/sdio.c
> index d3dd7bf..317796b 100644
> --- a/drivers/net/wireless/ti/wlcore/sdio.c
> +++ b/drivers/net/wireless/ti/wlcore/sdio.c
Please make this a two-patch series and keep the dt binding in a separate
patch from the driver change.
> +#ifdef CONFIG_OF
> +static struct wl12xx_platform_data *wlcore_probe_of(struct device *dev)
> +{
> + struct device_node *np = dev->of_node;
> + struct wl12xx_platform_data *pdata;
> +
> + if (!np || !of_device_is_compatible(np, "ti,wlcore")) {
> + dev_err(dev, "No platform data set\n");
> + return NULL;
> + }
> +
> + pdata = kzalloc(sizeof(*pdata), GFP_KERNEL);
> + if (!pdata)
> + return NULL;
Your method seems overly complicated. While a lot of drivers do the same
thing, allocating a platform_data structure at probe time is really
just extra work compared to making the platform_data optional and
adding the required fields into the driver-private structure.
Arnd
--
To unsubscribe from this list: send the line "unsubscribe linux-wireless" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
