Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1588924 > unrolled thread

[RFC 0/3] drivers: Add an API to read device specific config data

Back to article view | Back to linux.kernel

Contents

  [RFC 0/3] drivers: Add an API to read device specific config data Alban <albeu@free.fr> - 2017-02-27 21:40 +0100
    [PATCH 3/3] ath9k: ahb: Add OF support Alban <albeu@free.fr> - 2017-02-27 21:50 +0100
      Re: [PATCH 3/3] ath9k: ahb: Add OF support Rafał Miłecki <zajec5@gmail.com> - 2017-02-27 22:20 +0100
        Re: [PATCH 3/3] ath9k: ahb: Add OF support Rafał Miłecki <zajec5@gmail.com> - 2017-02-28 00:00 +0100
        Re: [PATCH 3/3] ath9k: ahb: Add OF support Alban <albeu@free.fr> - 2017-02-28 00:20 +0100
    Re: [RFC 0/3] drivers: Add an API to read device specific config data Marek Vasut <marek.vasut@gmail.com> - 2017-02-27 21:50 +0100
    Re: [RFC 0/3] drivers: Add an API to read device specific config  data Boris Brezillon <boris.brezillon@free-electrons.com> - 2017-02-27 22:40 +0100
      Re: [RFC 0/3] drivers: Add an API to read device specific config data Moritz Fischer <moritz.fischer@ettus.com> - 2017-02-27 23:30 +0100
      Re: [RFC 0/3] drivers: Add an API to read device specific config  data Alban <albeu@free.fr> - 2017-02-27 23:50 +0100

#1588924 — [RFC 0/3] drivers: Add an API to read device specific config data

Hi all,

while looking at adding OF support for the ath9k driver I had the problem of
reading the EEPROM data. On the SoC platforms this data is stored in an SPI
flash along with a few other things. In OpenWRT/LEDE this data is read from
the board init code using the fact that the flash is (normaly) readable from
a memory map. A bit too hackish for my taste.

This is just one example, there is various other similar cases, mostly with
MAC addresses. I thought it would be nicer if we had a clean API for this,
similar to the firmware API but per device instance instead of beeing per
driver. The device driver wouldn't have to care where the data is stored,
they just request it and the backend take care of reading the EEPROM, MTD
or whatever is used on the board.

This series implement such an API along with an implementation for MTD
devices and a use in the ath9k driver. As this is an RFC I didn't yet
write the OF binding documentation, that will come later if the feedback
is positive.

Alban Bedel (3):
  drivers: Add an API to read device specific config data
  mtd: Add support for reading device data out of MTD devices
  ath9k: ahb: Add OF support

 drivers/base/Kconfig                   |   6 +
 drivers/base/Makefile                  |   1 +
 drivers/base/devdata.c                 | 204 +++++++++++++++++++++++++++++++++
 drivers/mtd/Kconfig                    |   9 ++
 drivers/mtd/Makefile                   |   1 +
 drivers/mtd/devdata.c                  |  70 +++++++++++
 drivers/net/wireless/ath/ath9k/Kconfig |   1 +
 drivers/net/wireless/ath/ath9k/ahb.c   |  55 +++++++--
 drivers/net/wireless/ath/ath9k/init.c  |  41 ++++++-
 include/linux/devdata.h                |  79 +++++++++++++
 10 files changed, 459 insertions(+), 8 deletions(-)
 create mode 100644 drivers/base/devdata.c
 create mode 100644 drivers/mtd/devdata.c
 create mode 100644 include/linux/devdata.h

-- 
2.7.4

[toc] | [next] | [standalone]

#1588933 — [PATCH 3/3] ath9k: ahb: Add OF support

From: Alban Bedel <albeu@free.fr>

Allow registering ath9k AHB devices defined in OF. The binding
currently only allow to set the MAC address and to optionally
disable the 2GHz or 5GHz band. The EEPROM data is loaded using
the device data API.

Signed-off-by: Alban Bedel <albeu@free.fr>
---
 drivers/net/wireless/ath/ath9k/Kconfig |  1 +
 drivers/net/wireless/ath/ath9k/ahb.c   | 55 +++++++++++++++++++++++++++++-----
 drivers/net/wireless/ath/ath9k/init.c  | 41 ++++++++++++++++++++++++-
 3 files changed, 89 insertions(+), 8 deletions(-)

diff --git a/drivers/net/wireless/ath/ath9k/Kconfig b/drivers/net/wireless/ath/ath9k/Kconfig
index 783a38f..5d459f7 100644
--- a/drivers/net/wireless/ath/ath9k/Kconfig
+++ b/drivers/net/wireless/ath/ath9k/Kconfig
@@ -49,6 +49,7 @@ config ATH9K_PCI
 config ATH9K_AHB
 	bool "Atheros ath9k AHB bus support"
 	depends on ATH9K
+	select DEVDATA
 	default n
 	---help---
 	  This option enables the AHB bus support in ath9k.
diff --git a/drivers/net/wireless/ath/ath9k/ahb.c b/drivers/net/wireless/ath/ath9k/ahb.c
index 2bd982c..5aaaa16 100644
--- a/drivers/net/wireless/ath/ath9k/ahb.c
+++ b/drivers/net/wireless/ath/ath9k/ahb.c
@@ -18,6 +18,9 @@
 
 #include <linux/nl80211.h>
 #include <linux/platform_device.h>
+#include <linux/devdata.h>
+#include <linux/of_device.h>
+#include <linux/of_net.h>
 #include <linux/module.h>
 #include "ath9k.h"
 
@@ -49,6 +52,33 @@ static const struct platform_device_id ath9k_platform_id_table[] = {
 	{},
 };
 
+#ifdef CONFIG_OF
+static const struct of_device_id ath_ahb_of_match[] = {
+	{
+		.compatible = "qca,ar9100-wmac",
+		.data = (void *)AR5416_AR9100_DEVID
+	},
+	{
+		.compatible = "qca,ar9330-wmac",
+		.data = (void *)AR9300_DEVID_AR9330
+	},
+	{
+		.compatible = "qca,ar9340-wmac",
+		.data = (void *)AR9300_DEVID_AR9340
+	},
+	{
+		.compatible = "qca,qca9550-wmac",
+		.data = (void *)AR9300_DEVID_QCA955X
+	},
+	{
+		.compatible = "qca,qca9530-wmac",
+		.data = (void *)AR9300_DEVID_AR953X
+	},
+	{ },
+};
+MODULE_DEVICE_TABLE(of, ath_ahb_of_match);
+#endif
+
 /* return bus cachesize in 4B word units */
 static void ath_ahb_read_cachesize(struct ath_common *common, int *csz)
 {
@@ -57,9 +87,9 @@ static void ath_ahb_read_cachesize(struct ath_common *common, int *csz)
 
 static bool ath_ahb_eeprom_read(struct ath_common *common, u32 off, u16 *data)
 {
-	ath_err(common, "%s: eeprom data has to be provided externally\n",
-		__func__);
-	return false;
+	struct ath_softc *sc = (struct ath_softc *)common->priv;
+
+	return !devdata_read(sc->dev, "eeprom", off * 2, data, 2);
 }
 
 static const struct ath_bus_ops ath_ahb_bus_ops  = {
@@ -79,10 +109,20 @@ static int ath_ahb_probe(struct platform_device *pdev)
 	int ret = 0;
 	struct ath_hw *ah;
 	char hw_name[64];
+	u16 devid;
 
-	if (!dev_get_platdata(&pdev->dev)) {
-		dev_err(&pdev->dev, "no platform data specified\n");
-		return -EINVAL;
+	if (id) {
+		devid = id->driver_data;
+	} else {
+		const struct of_device_id *match;
+
+		match = of_match_device(ath_ahb_of_match, &pdev->dev);
+		if (!match) {
+			dev_err(&pdev->dev, "no device match found\n");
+			return -EINVAL;
+		}
+
+		devid = (u16)(unsigned long)match->data;
 	}
 
 	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
@@ -127,7 +167,7 @@ static int ath_ahb_probe(struct platform_device *pdev)
 		goto err_free_hw;
 	}
 
-	ret = ath9k_init_device(id->driver_data, sc, &ath_ahb_bus_ops);
+	ret = ath9k_init_device(devid, sc, &ath_ahb_bus_ops);
 	if (ret) {
 		dev_err(&pdev->dev, "failed to initialize device\n");
 		goto err_irq;
@@ -167,6 +207,7 @@ static struct platform_driver ath_ahb_driver = {
 	.remove     = ath_ahb_remove,
 	.driver		= {
 		.name	= "ath9k",
+		.of_match_table = of_match_ptr(ath_ahb_of_match),
 	},
 	.id_table    = ath9k_platform_id_table,
 };
diff --git a/drivers/net/wireless/ath/ath9k/init.c b/drivers/net/wireless/ath/ath9k/init.c
index fa4b3cc..21194e5 100644
--- a/drivers/net/wireless/ath/ath9k/init.c
+++ b/drivers/net/wireless/ath/ath9k/init.c
@@ -23,6 +23,8 @@
 #include <linux/of.h>
 #include <linux/of_net.h>
 #include <linux/relay.h>
+#include <linux/clk.h>
+#include <linux/of_net.h>
 #include <net/ieee80211_radiotap.h>
 
 #include "ath9k.h"
@@ -513,6 +515,43 @@ static void ath9k_eeprom_release(struct ath_softc *sc)
 	release_firmware(sc->sc_ah->eeprom_blob);
 }
 
+#ifdef CONFIG_OF
+static int ath9k_init_of(struct ath_softc *sc)
+{
+	struct device_node *np = sc->dev->of_node;
+	struct ath_hw *ah = sc->sc_ah;
+	const void *macaddr;
+	struct clk *clk;
+	int ret = 0;
+
+	if (!np) {
+		dev_err(sc->dev, "no platform data or OF node\n");
+		return -EINVAL;
+	}
+
+	clk = clk_get(sc->dev, "ref");
+	if (!IS_ERR(clk)) {
+		ah->is_clk_25mhz = (clk_get_rate(clk) == 25000000);
+		clk_put(clk);
+	}
+
+	ah->disable_2ghz = of_property_read_bool(np, "qca,disable-2ghz");
+	ah->disable_5ghz = of_property_read_bool(np, "qca,disable-5ghz");
+
+	macaddr = of_get_mac_address(np);
+	if (macaddr)
+		memcpy(ath9k_hw_common(ah)->macaddr, macaddr, ETH_ALEN);
+
+	return ret;
+}
+#else
+static int ath9k_init_of(struct ath_softc *sc)
+{
+	dev_err(sc->dev, "no platform data\n");
+	return -EINVAL;
+}
+#endif
+
 static int ath9k_init_platform(struct ath_softc *sc)
 {
 	struct ath9k_platform_data *pdata = sc->dev->platform_data;
@@ -521,7 +560,7 @@ static int ath9k_init_platform(struct ath_softc *sc)
 	int ret;
 
 	if (!pdata)
-		return 0;
+		return ath9k_init_of(sc);
 
 	if (!pdata->use_eeprom) {
 		ah->ah_flags &= ~AH_USE_EEPROM;
-- 
2.7.4

[toc] | [prev] | [next] | [standalone]

#1588948 — Re: [PATCH 3/3] ath9k: ahb: Add OF support

Why you didn't cc linux-wireless?!?!

On 27 February 2017 at 21:28, Alban <albeu@free.fr> wrote:
> @@ -513,6 +515,43 @@ static void ath9k_eeprom_release(struct ath_softc *sc)
>         release_firmware(sc->sc_ah->eeprom_blob);
>  }
>
> +#ifdef CONFIG_OF
> +static int ath9k_init_of(struct ath_softc *sc)
> +{
> +       struct device_node *np = sc->dev->of_node;
> +       struct ath_hw *ah = sc->sc_ah;
> +       const void *macaddr;
> +       struct clk *clk;
> +       int ret = 0;
> +
> +       if (!np) {
> +               dev_err(sc->dev, "no platform data or OF node\n");
> +               return -EINVAL;
> +       }
> +
> +       clk = clk_get(sc->dev, "ref");
> +       if (!IS_ERR(clk)) {
> +               ah->is_clk_25mhz = (clk_get_rate(clk) == 25000000);
> +               clk_put(clk);
> +       }
> +
> +       ah->disable_2ghz = of_property_read_bool(np, "qca,disable-2ghz");
> +       ah->disable_5ghz = of_property_read_bool(np, "qca,disable-5ghz");

Please use ieee80211-freq-limit:
https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/commit/?id=b330b25eaabda00d74e47566d9200907da381896

Most likely with the wiphy_read_of_freq_limits helper:
https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/commit/?id=e691ac2f75b69bee743f0370d79454ba4429b17

Example:
https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/commit/?id=0f83ff69735651cc7a3d150466a5257ff829b62b

[toc] | [prev] | [next] | [standalone]

#1589027 — Re: [PATCH 3/3] ath9k: ahb: Add OF support

On 27 February 2017 at 23:48, Alban <albeu@free.fr> wrote:
> On Mon, 27 Feb 2017 22:13:21 +0100
> Rafał Miłecki <zajec5@gmail.com> wrote:
>
>> Why you didn't cc linux-wireless?!?!
>
> I first wanted to be sure that the devdata part was generally
> acceptable, this patch was just included as an example of a user.
> But it sound like that part will have to move to nvmem first.
> I'll come back with a new patch once MTD support for nvmem is
> done.

OK, I just realized this was supposed to be RFC (for some reason this
patch didn't include RFC tag). At least this is was I assume to by
looking at the:
[RFC 0/3] drivers: Add an API to read device specific config data

Good luck!

[toc] | [prev] | [next] | [standalone]

#1589041 — Re: [PATCH 3/3] ath9k: ahb: Add OF support

[Multipart message — attachments visible in raw view] — view raw

On Mon, 27 Feb 2017 22:13:21 +0100
Rafał Miłecki <zajec5@gmail.com> wrote:

> Why you didn't cc linux-wireless?!?!

I first wanted to be sure that the devdata part was generally
acceptable, this patch was just included as an example of a user.
But it sound like that part will have to move to nvmem first.
I'll come back with a new patch once MTD support for nvmem is
done.

> On 27 February 2017 at 21:28, Alban <albeu@free.fr> wrote:
> > @@ -513,6 +515,43 @@ static void ath9k_eeprom_release(struct ath_softc *sc)
> >         release_firmware(sc->sc_ah->eeprom_blob);
> >  }
> >
> > +#ifdef CONFIG_OF
> > +static int ath9k_init_of(struct ath_softc *sc)
> > +{
> > +       struct device_node *np = sc->dev->of_node;
> > +       struct ath_hw *ah = sc->sc_ah;
> > +       const void *macaddr;
> > +       struct clk *clk;
> > +       int ret = 0;
> > +
> > +       if (!np) {
> > +               dev_err(sc->dev, "no platform data or OF node\n");
> > +               return -EINVAL;
> > +       }
> > +
> > +       clk = clk_get(sc->dev, "ref");
> > +       if (!IS_ERR(clk)) {
> > +               ah->is_clk_25mhz = (clk_get_rate(clk) == 25000000);
> > +               clk_put(clk);
> > +       }
> > +
> > +       ah->disable_2ghz = of_property_read_bool(np, "qca,disable-2ghz");
> > +       ah->disable_5ghz = of_property_read_bool(np, "qca,disable-5ghz");  
> 
> Please use ieee80211-freq-limit:
> https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/commit/?id=b330b25eaabda00d74e47566d9200907da381896
> 
> Most likely with the wiphy_read_of_freq_limits helper:
> https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/commit/?id=e691ac2f75b69bee743f0370d79454ba4429b17
> 
> Example:
> https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/commit/?id=0f83ff69735651cc7a3d150466a5257ff829b62b

Thanks, I'll check this.

Alban

[toc] | [prev] | [next] | [standalone]

#1588934

On 02/27/2017 09:28 PM, Alban wrote:
> Hi all,
> 
> while looking at adding OF support for the ath9k driver I had the problem of
> reading the EEPROM data. On the SoC platforms this data is stored in an SPI
> flash along with a few other things. In OpenWRT/LEDE this data is read from
> the board init code using the fact that the flash is (normaly) readable from
> a memory map. A bit too hackish for my taste.
> 
> This is just one example, there is various other similar cases, mostly with
> MAC addresses. I thought it would be nicer if we had a clean API for this,
> similar to the firmware API but per device instance instead of beeing per
> driver. The device driver wouldn't have to care where the data is stored,
> they just request it and the backend take care of reading the EEPROM, MTD
> or whatever is used on the board.

Can't we use the NVMEM for this stuff ?

> This series implement such an API along with an implementation for MTD
> devices and a use in the ath9k driver. As this is an RFC I didn't yet
> write the OF binding documentation, that will come later if the feedback
> is positive.
> 
> Alban Bedel (3):
>   drivers: Add an API to read device specific config data
>   mtd: Add support for reading device data out of MTD devices
>   ath9k: ahb: Add OF support
> 
>  drivers/base/Kconfig                   |   6 +
>  drivers/base/Makefile                  |   1 +
>  drivers/base/devdata.c                 | 204 +++++++++++++++++++++++++++++++++
>  drivers/mtd/Kconfig                    |   9 ++
>  drivers/mtd/Makefile                   |   1 +
>  drivers/mtd/devdata.c                  |  70 +++++++++++
>  drivers/net/wireless/ath/ath9k/Kconfig |   1 +
>  drivers/net/wireless/ath/ath9k/ahb.c   |  55 +++++++--
>  drivers/net/wireless/ath/ath9k/init.c  |  41 ++++++-
>  include/linux/devdata.h                |  79 +++++++++++++
>  10 files changed, 459 insertions(+), 8 deletions(-)
>  create mode 100644 drivers/base/devdata.c
>  create mode 100644 drivers/mtd/devdata.c
>  create mode 100644 include/linux/devdata.h
> 


-- 
Best regards,
Marek Vasut

[toc] | [prev] | [next] | [standalone]

#1588955 — Re: [RFC 0/3] drivers: Add an API to read device specific config data

+Moritz

Hi Alban,

On Mon, 27 Feb 2017 21:28:09 +0100
Alban <albeu@free.fr> wrote:

> Hi all,
> 
> while looking at adding OF support for the ath9k driver I had the problem of
> reading the EEPROM data. On the SoC platforms this data is stored in an SPI
> flash along with a few other things. In OpenWRT/LEDE this data is read from
> the board init code using the fact that the flash is (normaly) readable from
> a memory map. A bit too hackish for my taste.
> 
> This is just one example, there is various other similar cases, mostly with
> MAC addresses. I thought it would be nicer if we had a clean API for this,
> similar to the firmware API but per device instance instead of beeing per
> driver. The device driver wouldn't have to care where the data is stored,
> they just request it and the backend take care of reading the EEPROM, MTD
> or whatever is used on the board.
> 
> This series implement such an API along with an implementation for MTD
> devices and a use in the ath9k driver. As this is an RFC I didn't yet
> write the OF binding documentation, that will come later if the feedback
> is positive.

What you're looking for already exists and it's called nvmem[1]. Some
work has been done to expose MTD partitions as nvmem cells [2], but
it's never been finished.
Can you please finish Moritz implementation instead of creating a new
API?

Thanks,

Boris

[1]http://lxr.free-electrons.com/source/Documentation/nvmem/nvmem.txt
[2]https://patchwork.ozlabs.org/patch/626460/

[toc] | [prev] | [next] | [standalone]

#1588991

Hi all,

On Mon, Feb 27, 2017 at 12:42 PM, Boris Brezillon
<boris.brezillon@free-electrons.com> wrote:
> +Moritz
>
> Hi Alban,
>
> On Mon, 27 Feb 2017 21:28:09 +0100
> Alban <albeu@free.fr> wrote:
>
>> Hi all,
>>
>> while looking at adding OF support for the ath9k driver I had the problem of
>> reading the EEPROM data. On the SoC platforms this data is stored in an SPI
>> flash along with a few other things. In OpenWRT/LEDE this data is read from
>> the board init code using the fact that the flash is (normaly) readable from
>> a memory map. A bit too hackish for my taste.
>>
>> This is just one example, there is various other similar cases, mostly with
>> MAC addresses. I thought it would be nicer if we had a clean API for this,
>> similar to the firmware API but per device instance instead of beeing per
>> driver. The device driver wouldn't have to care where the data is stored,
>> they just request it and the backend take care of reading the EEPROM, MTD
>> or whatever is used on the board.
>>
>> This series implement such an API along with an implementation for MTD
>> devices and a use in the ath9k driver. As this is an RFC I didn't yet
>> write the OF binding documentation, that will come later if the feedback
>> is positive.
>
> What you're looking for already exists and it's called nvmem[1]. Some
> work has been done to expose MTD partitions as nvmem cells [2], but
> it's never been finished.
> Can you please finish Moritz implementation instead of creating a new
> API?

Whoops ... this totally fell off my radar (project got axed for $REASONS).
I could potentially take another look at this this weekend, otherwise I'm
happy to review whatever you come up with Alban.

Cheers,

Moritz

[toc] | [prev] | [next] | [standalone]

#1589014 — Re: [RFC 0/3] drivers: Add an API to read device specific config data

[Multipart message — attachments visible in raw view] — view raw

On Mon, 27 Feb 2017 21:42:30 +0100
Boris Brezillon <boris.brezillon@free-electrons.com> wrote:

> +Moritz
> 
> Hi Alban,
> 
> On Mon, 27 Feb 2017 21:28:09 +0100
> Alban <albeu@free.fr> wrote:
> 
> > Hi all,
> > 
> > while looking at adding OF support for the ath9k driver I had the problem of
> > reading the EEPROM data. On the SoC platforms this data is stored in an SPI
> > flash along with a few other things. In OpenWRT/LEDE this data is read from
> > the board init code using the fact that the flash is (normaly) readable from
> > a memory map. A bit too hackish for my taste.
> > 
> > This is just one example, there is various other similar cases, mostly with
> > MAC addresses. I thought it would be nicer if we had a clean API for this,
> > similar to the firmware API but per device instance instead of beeing per
> > driver. The device driver wouldn't have to care where the data is stored,
> > they just request it and the backend take care of reading the EEPROM, MTD
> > or whatever is used on the board.
> > 
> > This series implement such an API along with an implementation for MTD
> > devices and a use in the ath9k driver. As this is an RFC I didn't yet
> > write the OF binding documentation, that will come later if the feedback
> > is positive.  
> 
> What you're looking for already exists and it's called nvmem[1]. Some
> work has been done to expose MTD partitions as nvmem cells [2], but
> it's never been finished.
> Can you please finish Moritz implementation instead of creating a new
> API?

I didn't knew about the nvmem API :/ I'll look into finishing the MTD
support then.

Alban

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web