On Fri, Feb 07, 2014 at 07:48:49PM +0000, Marek Belisko wrote:
> Signed-off-by: NeilBrown <neilb@xxxxxxx>
> Signed-off-by: Marek Belisko <marek@xxxxxxxxxxxxx>
> ---
> Based on Neil's patch and extend for documentation and bindings include.
>
> .../bindings/net/rfkill/rfkill-relugator.txt | 28 ++++++++++++++++
> include/dt-bindings/net/rfkill-regulator.h | 23 +++++++++++++
> net/rfkill/rfkill-regulator.c | 38 ++++++++++++++++++++++
> 3 files changed, 89 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/net/rfkill/rfkill-relugator.txt
> create mode 100644 include/dt-bindings/net/rfkill-regulator.h
>
> diff --git a/Documentation/devicetree/bindings/net/rfkill/rfkill-relugator.txt b/Documentation/devicetree/bindings/net/rfkill/rfkill-relugator.txt
> new file mode 100644
> index 0000000..cdb7dd7
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/net/rfkill/rfkill-relugator.txt
> @@ -0,0 +1,28 @@
> +Regulator consumer for rfkill devices
What exactly is an "rfkill" device? How is it used? How does it relate
to other devices in the DT?
To me, this looks like a leak of a Linux abstraction.
> +
> +Required properties:
> +- compatible : Must be "rfkill-regulator".
> +- label : Name of rfkill device.
What's this for? Why does this need a label in the DT? Surely this can
be implied by the relationship to a particular radio device?
> +- type : Type of rfkill device.
> +
> +Possible values (defined in include/dt-bindings/net/rfkill-regulator.h):
> + RFKILL_TYPE_ALL
> + RFKILL_TYPE_WLAN
> + RFKILL_TYPE_BLUETOOTH
> + RFKILL_TYPE_UWB
> + RFKILL_TYPE_WIMAX
> + RFKILL_TYPE_WWAN
> + RFKILL_TYPE_GPS
> + RFKILL_TYPE_FM
> + RFKILL_TYPE_NFC
What do these mean? Why can these not be implied by a relationship to
any devices of these particular types?
> +
> +- vrfkill-supply - regulator device.
Why isn't this described on the radio revice node? It's a supply to the
radio, not to the rfkill concept.
> +
> +Example:
> + gps-rfkill {
> + compatible = "rfkill-regulator";
> + label = "GPS";
> + type = <RFKILL_TYPE_GPS>;
> + vrfkill-supply = <®>;
> + };
Why is this not bound to the particular GPS device in some way?
What if I have more than one of any of the types of device this
supports, which device is this expected to control?
Why is it described as a separate device in the device tree at all?
I do not think this binding is the right way to describe this.
Thanks,
Mark.
> +
> diff --git a/include/dt-bindings/net/rfkill-regulator.h b/include/dt-bindings/net/rfkill-regulator.h
> new file mode 100644
> index 0000000..ae32273
> --- /dev/null
> +++ b/include/dt-bindings/net/rfkill-regulator.h
> @@ -0,0 +1,23 @@
> +/*
> + * This header provides macros for rfkill-regulator bindings.
> + *
> + * Copyright (C) 2014 Marek Belisko <marek@xxxxxxxxxxxxx>
> + *
> + * GPLv2 only
> + */
> +
> +#ifndef __DT_BINDINGS_RFKILL_REGULATOR_H__
> +#define __DT_BINDINGS_RFKILL_REGULATOR_H__
> +
> +
> +#define RFKILL_TYPE_ALL (0)
> +#define RFKILL_TYPE_WLAN (1)
> +#define RFKILL_TYPE_BLUETOOTH (2)
> +#define RFKILL_TYPE_UWB (3)
> +#define RFKILL_TYPE_WIMAX (4)
> +#define RFKILL_TYPE_WWAN (5)
> +#define RFKILL_TYPE_GPS (6)
> +#define RFKILL_TYPE_FM (7)
> +#define RFKILL_TYPE_NFC (8)
> +
> +#endif /* __DT_BINDINGS_RFKILL_REGULATOR_H__ */
> diff --git a/net/rfkill/rfkill-regulator.c b/net/rfkill/rfkill-regulator.c
> index cf5b145..a04aff8 100644
> --- a/net/rfkill/rfkill-regulator.c
> +++ b/net/rfkill/rfkill-regulator.c
> @@ -19,6 +19,7 @@
> #include <linux/regulator/consumer.h>
> #include <linux/rfkill.h>
> #include <linux/rfkill-regulator.h>
> +#include <linux/of_platform.h>
>
> struct rfkill_regulator_data {
> struct rfkill *rf_kill;
> @@ -57,6 +58,31 @@ static struct rfkill_ops rfkill_regulator_ops = {
> .set_block = rfkill_regulator_set_block,
> };
>
> +#ifdef CONFIG_OF
> +static struct rfkill_regulator_platform_data *
> +rfkill_regulator_parse_pdata(struct device *dev)
> +{
> + struct rfkill_regulator_platform_data *pdata;
> + struct device_node *np = dev->of_node;
> + u32 num;
> + if (!np)
> + return NULL;
> + pdata = devm_kzalloc(dev, sizeof(*pdata), GFP_KERNEL);
> + if (!pdata)
> + return NULL;
> + if (of_property_read_u32(np, "type", &num) == 0)
> + pdata->type = num;
> + of_property_read_string(np, "label", &pdata->name);
> + return pdata;
> +}
> +#else
> +static inline struct rfkill_regulator_platform_data *
> +rfkill_regulator_parse_pdata(struct device *dev)
> +{
> + return NULL;
> +}
> +#endif
> +
> static int rfkill_regulator_probe(struct platform_device *pdev)
> {
> struct rfkill_regulator_platform_data *pdata = pdev->dev.platform_data;
> @@ -65,6 +91,9 @@ static int rfkill_regulator_probe(struct platform_device *pdev)
> struct rfkill *rf_kill;
> int ret = 0;
>
> + if (!pdata)
> + pdata = rfkill_regulator_parse_pdata(&pdev->dev);
> +
> if (pdata == NULL) {
> dev_err(&pdev->dev, "no platform data\n");
> return -ENODEV;
> @@ -137,12 +166,21 @@ static int rfkill_regulator_remove(struct platform_device *pdev)
> return 0;
> }
>
> +#ifdef CONFIG_OF
> +static const struct of_device_id rfkill_regulator_match[] = {
> + {.compatible = "rfkill-regulator"},
> + {}
> +};
> +MODULE_DEVICE_TABLE(of, rfkill_regulator_match);
> +#endif
> +
> static struct platform_driver rfkill_regulator_driver = {
> .probe = rfkill_regulator_probe,
> .remove = rfkill_regulator_remove,
> .driver = {
> .name = "rfkill-regulator",
> .owner = THIS_MODULE,
> + .of_match_table = of_match_ptr(rfkill_regulator_match),
> },
> };
>
> --
> 1.8.3.2
>
>
--
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
