Re: [PATCH 2/2] phylib: make mdio-gpio work without OF
On Fri, Oct 31, 2008 at 12:49, Paulius Zaleckas wrote: > +#ifndef NO_IRQ > +#define NO_IRQ 0 > +#endif discussions elsewhere indicate this should die. -if (irq != NO_IRQ) +if (irq) -mike -- To unsubscribe from this list: send the line "unsubscribe linux-embedded" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] phylib: mdio-ofgpio ---> mdio-gpio
On Friday 31 October 2008, Paulius Zaleckas wrote: > - if NO_IRQ is not defined define it to 0 Better yet, stop using that symbol entirely... -- To unsubscribe from this list: send the line "unsubscribe linux-embedded" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2/2] phylib: make mdio-gpio work without OF
On Fri, Oct 31, 2008 at 10:49 AM, Paulius Zaleckas
<[EMAIL PROTECTED]> wrote:
> make mdio-gpio work with non OpenFirmware gpio implementation.
>
Looking good, but it has too many #ifdef blocks for my taste. In
particular, if the driver is refactored a bit then all the OF stuff
can be contained within a single ifdef block, as can all the non-OF
stuff. Comments below...
> +#ifdef CONFIG_OF_GPIO
> static int __devinit mdio_ofgpio_bitbang_init(struct mii_bus *bus,
> struct device_node *np)
> {
> @@ -87,34 +102,60 @@ static int __devinit mdio_ofgpio_bitbang_init(struct
> mii_bus *bus,
>snprintf(bus->id, MII_BUS_ID_SIZE, "%x", bitbang->mdc);
>return 0;
> }
> -
> -static void __devinit add_phy(struct mii_bus *bus, struct device_node *np)
> +#else
> +static void __devinit mdio_gpio_bitbang_init(struct mii_bus *bus,
> +struct mdio_gpio_platform_data
> *pdata,
> + int bus_id)
> {
> - const u32 *data;
> - int len, id, irq;
> + struct mdio_gpio_info *bitbang = bus->priv;
> +
> + bitbang->mdc = pdata->mdc;
> + bitbang->mdio = pdata->mdio;
> +
> + snprintf(bus->id, MII_BUS_ID_SIZE, "phy%i", bus_id);
> +}
> +#endif
mdio_ofgpio_bitbang_init() is such short function and it is only
called once inside the probe() function. Rather than duplicate it, it
can probably be moved inside the OF probe function and do the same
thing for the non-OF probe().
>
> - data = of_get_property(np, "reg", &len);
> - if (!data || len != 4)
> +static void __devinit add_phy(struct mii_bus *bus, unsigned int phy_addr,
> + int phy_irq)
> +{
> + if (phy_addr >= PHY_MAX_ADDR) {
> + dev_err(bus->parent,
> + "Failed to add phy with invalid address: 0x%x",
> + phy_addr);
>return;
> + }
>
> - id = *data;
> - bus->phy_mask &= ~(1 << id);
> + bus->phy_mask &= ~(1 << phy_addr);
>
> - irq = of_irq_to_resource(np, 0, NULL);
> - if (irq != NO_IRQ)
> - bus->irq[id] = irq;
> + if (phy_irq != NO_IRQ)
> + bus->irq[phy_addr] = phy_irq;
> }
I like the refactoring of add_phy
>
> +#ifdef CONFIG_OF_GPIO
> static int __devinit mdio_ofgpio_probe(struct of_device *ofdev,
> const struct of_device_id *match)
> {
>struct device_node *np = NULL;
> + struct device *dev = &ofdev->dev;
> +#else
> +static int __devinit mdio_gpio_probe(struct platform_device *pdev)
> +{
> + struct mdio_gpio_platform_data *pdata;
> + struct device *dev = &pdev->dev;
> +#endif
Instead of doing multiple #ifdef sections throughout the probe
function, use one #ifdef block for the OF stuff and another for all
the non-OF stuff. You can factor out any non-trivial common code
blocks into shared helper functions.
Otherwise, looking good.
Thanks,
g.
--
Grant Likely, B.Sc., P.Eng.
Secret Lab Technologies Ltd.
--
To unsubscribe from this list: send the line "unsubscribe linux-embedded" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/2] phylib: rename mdio-ofgpio to mdio-gpio
On Fri, Oct 31, 2008 at 10:49 AM, Paulius Zaleckas
<[EMAIL PROTECTED]> wrote:
> Signed-off-by: Paulius Zaleckas <[EMAIL PROTECTED]>
> Cc: Laurent Pinchart <[EMAIL PROTECTED]>
> Cc: Grant Likely <[EMAIL PROTECTED]>
> Cc: Mike Frysinger <[EMAIL PROTECTED]>
Acked-by: Grant Likely <[EMAIL PROTECTED]>
> ---
>
> drivers/net/phy/Kconfig |2
> drivers/net/phy/Makefile |2
> drivers/net/phy/mdio-gpio.c | 204
> +
> drivers/net/phy/mdio-ofgpio.c | 204
> -
> 4 files changed, 206 insertions(+), 206 deletions(-)
> create mode 100644 drivers/net/phy/mdio-gpio.c
> delete mode 100644 drivers/net/phy/mdio-ofgpio.c
>
>
> diff --git a/drivers/net/phy/Kconfig b/drivers/net/phy/Kconfig
> index d55932a..0318077 100644
> --- a/drivers/net/phy/Kconfig
> +++ b/drivers/net/phy/Kconfig
> @@ -84,7 +84,7 @@ config MDIO_BITBANG
>
> If in doubt, say N.
>
> -config MDIO_OF_GPIO
> +config MDIO_GPIO
>tristate "Support for GPIO lib-based bitbanged MDIO buses"
>depends on MDIO_BITBANG && OF_GPIO
>---help---
> diff --git a/drivers/net/phy/Makefile b/drivers/net/phy/Makefile
> index eee329f..9ae5d30 100644
> --- a/drivers/net/phy/Makefile
> +++ b/drivers/net/phy/Makefile
> @@ -15,4 +15,4 @@ obj-$(CONFIG_ICPLUS_PHY) += icplus.o
> obj-$(CONFIG_REALTEK_PHY) += realtek.o
> obj-$(CONFIG_FIXED_PHY)+= fixed.o
> obj-$(CONFIG_MDIO_BITBANG) += mdio-bitbang.o
> -obj-$(CONFIG_MDIO_OF_GPIO) += mdio-ofgpio.o
> +obj-$(CONFIG_MDIO_GPIO)+= mdio-gpio.o
> diff --git a/drivers/net/phy/mdio-gpio.c b/drivers/net/phy/mdio-gpio.c
> new file mode 100644
> index 000..2ff9775
> --- /dev/null
> +++ b/drivers/net/phy/mdio-gpio.c
> @@ -0,0 +1,204 @@
> +/*
> + * OpenFirmware GPIO based MDIO bitbang driver.
> + *
> + * Copyright (c) 2008 CSE Semaphore Belgium.
> + * by Laurent Pinchart <[EMAIL PROTECTED]>
> + *
> + * Based on earlier work by
> + *
> + * Copyright (c) 2003 Intracom S.A.
> + * by Pantelis Antoniou <[EMAIL PROTECTED]>
> + *
> + * 2005 (c) MontaVista Software, Inc.
> + * Vitaly Bordug <[EMAIL PROTECTED]>
> + *
> + * This file is licensed under the terms of the GNU General Public License
> + * version 2. This program is licensed "as is" without any warranty of any
> + * kind, whether express or implied.
> + */
> +
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +
> +struct mdio_gpio_info {
> + struct mdiobb_ctrl ctrl;
> + int mdc, mdio;
> +};
> +
> +static void mdio_dir(struct mdiobb_ctrl *ctrl, int dir)
> +{
> + struct mdio_gpio_info *bitbang =
> + container_of(ctrl, struct mdio_gpio_info, ctrl);
> +
> + if (dir)
> + gpio_direction_output(bitbang->mdio, 1);
> + else
> + gpio_direction_input(bitbang->mdio);
> +}
> +
> +static int mdio_read(struct mdiobb_ctrl *ctrl)
> +{
> + struct mdio_gpio_info *bitbang =
> + container_of(ctrl, struct mdio_gpio_info, ctrl);
> +
> + return gpio_get_value(bitbang->mdio);
> +}
> +
> +static void mdio(struct mdiobb_ctrl *ctrl, int what)
> +{
> + struct mdio_gpio_info *bitbang =
> + container_of(ctrl, struct mdio_gpio_info, ctrl);
> +
> + gpio_set_value(bitbang->mdio, what);
> +}
> +
> +static void mdc(struct mdiobb_ctrl *ctrl, int what)
> +{
> + struct mdio_gpio_info *bitbang =
> + container_of(ctrl, struct mdio_gpio_info, ctrl);
> +
> + gpio_set_value(bitbang->mdc, what);
> +}
> +
> +static struct mdiobb_ops mdio_gpio_ops = {
> + .owner = THIS_MODULE,
> + .set_mdc = mdc,
> + .set_mdio_dir = mdio_dir,
> + .set_mdio_data = mdio,
> + .get_mdio_data = mdio_read,
> +};
> +
> +static int __devinit mdio_ofgpio_bitbang_init(struct mii_bus *bus,
> + struct device_node *np)
> +{
> + struct mdio_gpio_info *bitbang = bus->priv;
> +
> + bitbang->mdc = of_get_gpio(np, 0);
> + bitbang->mdio = of_get_gpio(np, 1);
> +
> + if (bitbang->mdc < 0 || bitbang->mdio < 0)
> + return -ENODEV;
> +
> + snprintf(bus->id, MII_BUS_ID_SIZE, "%x", bitbang->mdc);
> + return 0;
> +}
> +
> +static void __devinit add_phy(struct mii_bus *bus, struct device_node *np)
> +{
> + const u32 *data;
> + int len, id, irq;
> +
> + data = of_get_property(np, "reg", &len);
> + if (!data || len != 4)
> + return;
> +
> + id = *data;
> + bus->phy_mask &= ~(1 << id);
> +
> + irq = of_irq_to_resource(np, 0, NULL);
> + if (irq != NO_IRQ)
> + bus->irq[id] = irq;
> +}
> +
> +static int __devinit mdio_ofgpio_probe(struct of_device *ofdev,
> +const struct of_device_id *match)
> +{
> + struct device_node *np = NULL;
> + struct mii_bus *new_bus;
> + str
Re: [PATCH] phylib: mdio-ofgpio ---> mdio-gpio
On Fri, Oct 31, 2008 at 10:06:26AM -0600, Grant Likely wrote: > On Fri, Oct 31, 2008 at 9:53 AM, Paulius Zaleckas > <[EMAIL PROTECTED]> wrote: > > Rename mdio-ofgpio driver to mdio-gpio and make it work with > > non OpenFirmware gpio implementation. > > Please respin this patch in 2 pieces; 1 patch to move the file and 1 > patch to make the changes. It is difficult to review as a single > patch. Or ask for the diffs to be in git format (iow, using git diff -M). Don't know if git-send-email can do that. -- To unsubscribe from this list: send the line "unsubscribe linux-embedded" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 2/2] phylib: make mdio-gpio work without OF
make mdio-gpio work with non OpenFirmware gpio implementation.
Aditional changes to mdio-gpio:
- use gpio_request() and gpio_free()
- place irq[] array in struct mdio_gpio_info
- add module description, author and license
- if NO_IRQ is not defined define it to 0
- add note about compiling this driver as module
- rename mdc and mdio function (were ugly names)
- change MII to MDIO in bus name
- add __init __exit to module (un)loading functions
- probe fails if no phys added to the bus
- kzalloc bitbang with sizeof(*bitbang)
Laurent, please test this driver under OF.
Signed-off-by: Paulius Zaleckas <[EMAIL PROTECTED]>
Cc: Laurent Pinchart <[EMAIL PROTECTED]>
Cc: Grant Likely <[EMAIL PROTECTED]>
Cc: Mike Frysinger <[EMAIL PROTECTED]>
---
drivers/net/phy/Kconfig |5 +
drivers/net/phy/mdio-gpio.c | 188 +--
include/linux/mdio-gpio.h | 30 +++
3 files changed, 180 insertions(+), 43 deletions(-)
create mode 100644 include/linux/mdio-gpio.h
diff --git a/drivers/net/phy/Kconfig b/drivers/net/phy/Kconfig
index 0318077..c4c5a2f 100644
--- a/drivers/net/phy/Kconfig
+++ b/drivers/net/phy/Kconfig
@@ -86,8 +86,11 @@ config MDIO_BITBANG
config MDIO_GPIO
tristate "Support for GPIO lib-based bitbanged MDIO buses"
- depends on MDIO_BITBANG && OF_GPIO
+ depends on MDIO_BITBANG && GENERIC_GPIO
---help---
Supports GPIO lib-based MDIO busses.
+ To compile this driver as a module, choose M here: the module
+ will be called mdio-gpio.
+
endif # PHYLIB
diff --git a/drivers/net/phy/mdio-gpio.c b/drivers/net/phy/mdio-gpio.c
index 2ff9775..a7b94ac 100644
--- a/drivers/net/phy/mdio-gpio.c
+++ b/drivers/net/phy/mdio-gpio.c
@@ -1,9 +1,12 @@
/*
- * OpenFirmware GPIO based MDIO bitbang driver.
+ * GPIO based MDIO bitbang driver.
+ * Supports OpenFirmware.
*
* Copyright (c) 2008 CSE Semaphore Belgium.
* by Laurent Pinchart <[EMAIL PROTECTED]>
*
+ * Copyright (C) 2008, Paulius Zaleckas <[EMAIL PROTECTED]>
+ *
* Based on earlier work by
*
* Copyright (c) 2003 Intracom S.A.
@@ -22,12 +25,23 @@
#include
#include
#include
-#include
-#include
+#ifdef CONFIG_OF_GPIO
+# include
+# include
+#else
+# include
+# include
+# include
+#endif
+
+#ifndef NO_IRQ
+#define NO_IRQ 0
+#endif
struct mdio_gpio_info {
struct mdiobb_ctrl ctrl;
int mdc, mdio;
+ int irq[PHY_MAX_ADDR];
};
static void mdio_dir(struct mdiobb_ctrl *ctrl, int dir)
@@ -41,7 +55,7 @@ static void mdio_dir(struct mdiobb_ctrl *ctrl, int dir)
gpio_direction_input(bitbang->mdio);
}
-static int mdio_read(struct mdiobb_ctrl *ctrl)
+static int mdio_get(struct mdiobb_ctrl *ctrl)
{
struct mdio_gpio_info *bitbang =
container_of(ctrl, struct mdio_gpio_info, ctrl);
@@ -49,7 +63,7 @@ static int mdio_read(struct mdiobb_ctrl *ctrl)
return gpio_get_value(bitbang->mdio);
}
-static void mdio(struct mdiobb_ctrl *ctrl, int what)
+static void mdio_set(struct mdiobb_ctrl *ctrl, int what)
{
struct mdio_gpio_info *bitbang =
container_of(ctrl, struct mdio_gpio_info, ctrl);
@@ -57,7 +71,7 @@ static void mdio(struct mdiobb_ctrl *ctrl, int what)
gpio_set_value(bitbang->mdio, what);
}
-static void mdc(struct mdiobb_ctrl *ctrl, int what)
+static void mdc_set(struct mdiobb_ctrl *ctrl, int what)
{
struct mdio_gpio_info *bitbang =
container_of(ctrl, struct mdio_gpio_info, ctrl);
@@ -67,12 +81,13 @@ static void mdc(struct mdiobb_ctrl *ctrl, int what)
static struct mdiobb_ops mdio_gpio_ops = {
.owner = THIS_MODULE,
- .set_mdc = mdc,
+ .set_mdc = mdc_set,
.set_mdio_dir = mdio_dir,
- .set_mdio_data = mdio,
- .get_mdio_data = mdio_read,
+ .set_mdio_data = mdio_set,
+ .get_mdio_data = mdio_get,
};
+#ifdef CONFIG_OF_GPIO
static int __devinit mdio_ofgpio_bitbang_init(struct mii_bus *bus,
struct device_node *np)
{
@@ -87,34 +102,60 @@ static int __devinit mdio_ofgpio_bitbang_init(struct
mii_bus *bus,
snprintf(bus->id, MII_BUS_ID_SIZE, "%x", bitbang->mdc);
return 0;
}
-
-static void __devinit add_phy(struct mii_bus *bus, struct device_node *np)
+#else
+static void __devinit mdio_gpio_bitbang_init(struct mii_bus *bus,
+struct mdio_gpio_platform_data *pdata,
+ int bus_id)
{
- const u32 *data;
- int len, id, irq;
+ struct mdio_gpio_info *bitbang = bus->priv;
+
+ bitbang->mdc = pdata->mdc;
+ bitbang->mdio = pdata->mdio;
+
+ snprintf(bus->id, MII_BUS_ID_SIZE, "phy%i", bus_id);
+}
+#endif
- data = of_get_property(np, "reg", &len);
- if (!data || len != 4)
+static void __devinit add_phy(struct mii_bus *bus, unsigned int phy_addr,
+ int phy_irq)
+{
+ if (phy_addr >= PHY
[PATCH 1/2] phylib: rename mdio-ofgpio to mdio-gpio
Signed-off-by: Paulius Zaleckas <[EMAIL PROTECTED]>
Cc: Laurent Pinchart <[EMAIL PROTECTED]>
Cc: Grant Likely <[EMAIL PROTECTED]>
Cc: Mike Frysinger <[EMAIL PROTECTED]>
---
drivers/net/phy/Kconfig |2
drivers/net/phy/Makefile |2
drivers/net/phy/mdio-gpio.c | 204 +
drivers/net/phy/mdio-ofgpio.c | 204 -
4 files changed, 206 insertions(+), 206 deletions(-)
create mode 100644 drivers/net/phy/mdio-gpio.c
delete mode 100644 drivers/net/phy/mdio-ofgpio.c
diff --git a/drivers/net/phy/Kconfig b/drivers/net/phy/Kconfig
index d55932a..0318077 100644
--- a/drivers/net/phy/Kconfig
+++ b/drivers/net/phy/Kconfig
@@ -84,7 +84,7 @@ config MDIO_BITBANG
If in doubt, say N.
-config MDIO_OF_GPIO
+config MDIO_GPIO
tristate "Support for GPIO lib-based bitbanged MDIO buses"
depends on MDIO_BITBANG && OF_GPIO
---help---
diff --git a/drivers/net/phy/Makefile b/drivers/net/phy/Makefile
index eee329f..9ae5d30 100644
--- a/drivers/net/phy/Makefile
+++ b/drivers/net/phy/Makefile
@@ -15,4 +15,4 @@ obj-$(CONFIG_ICPLUS_PHY) += icplus.o
obj-$(CONFIG_REALTEK_PHY) += realtek.o
obj-$(CONFIG_FIXED_PHY)+= fixed.o
obj-$(CONFIG_MDIO_BITBANG) += mdio-bitbang.o
-obj-$(CONFIG_MDIO_OF_GPIO) += mdio-ofgpio.o
+obj-$(CONFIG_MDIO_GPIO)+= mdio-gpio.o
diff --git a/drivers/net/phy/mdio-gpio.c b/drivers/net/phy/mdio-gpio.c
new file mode 100644
index 000..2ff9775
--- /dev/null
+++ b/drivers/net/phy/mdio-gpio.c
@@ -0,0 +1,204 @@
+/*
+ * OpenFirmware GPIO based MDIO bitbang driver.
+ *
+ * Copyright (c) 2008 CSE Semaphore Belgium.
+ * by Laurent Pinchart <[EMAIL PROTECTED]>
+ *
+ * Based on earlier work by
+ *
+ * Copyright (c) 2003 Intracom S.A.
+ * by Pantelis Antoniou <[EMAIL PROTECTED]>
+ *
+ * 2005 (c) MontaVista Software, Inc.
+ * Vitaly Bordug <[EMAIL PROTECTED]>
+ *
+ * This file is licensed under the terms of the GNU General Public License
+ * version 2. This program is licensed "as is" without any warranty of any
+ * kind, whether express or implied.
+ */
+
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+
+struct mdio_gpio_info {
+ struct mdiobb_ctrl ctrl;
+ int mdc, mdio;
+};
+
+static void mdio_dir(struct mdiobb_ctrl *ctrl, int dir)
+{
+ struct mdio_gpio_info *bitbang =
+ container_of(ctrl, struct mdio_gpio_info, ctrl);
+
+ if (dir)
+ gpio_direction_output(bitbang->mdio, 1);
+ else
+ gpio_direction_input(bitbang->mdio);
+}
+
+static int mdio_read(struct mdiobb_ctrl *ctrl)
+{
+ struct mdio_gpio_info *bitbang =
+ container_of(ctrl, struct mdio_gpio_info, ctrl);
+
+ return gpio_get_value(bitbang->mdio);
+}
+
+static void mdio(struct mdiobb_ctrl *ctrl, int what)
+{
+ struct mdio_gpio_info *bitbang =
+ container_of(ctrl, struct mdio_gpio_info, ctrl);
+
+ gpio_set_value(bitbang->mdio, what);
+}
+
+static void mdc(struct mdiobb_ctrl *ctrl, int what)
+{
+ struct mdio_gpio_info *bitbang =
+ container_of(ctrl, struct mdio_gpio_info, ctrl);
+
+ gpio_set_value(bitbang->mdc, what);
+}
+
+static struct mdiobb_ops mdio_gpio_ops = {
+ .owner = THIS_MODULE,
+ .set_mdc = mdc,
+ .set_mdio_dir = mdio_dir,
+ .set_mdio_data = mdio,
+ .get_mdio_data = mdio_read,
+};
+
+static int __devinit mdio_ofgpio_bitbang_init(struct mii_bus *bus,
+ struct device_node *np)
+{
+ struct mdio_gpio_info *bitbang = bus->priv;
+
+ bitbang->mdc = of_get_gpio(np, 0);
+ bitbang->mdio = of_get_gpio(np, 1);
+
+ if (bitbang->mdc < 0 || bitbang->mdio < 0)
+ return -ENODEV;
+
+ snprintf(bus->id, MII_BUS_ID_SIZE, "%x", bitbang->mdc);
+ return 0;
+}
+
+static void __devinit add_phy(struct mii_bus *bus, struct device_node *np)
+{
+ const u32 *data;
+ int len, id, irq;
+
+ data = of_get_property(np, "reg", &len);
+ if (!data || len != 4)
+ return;
+
+ id = *data;
+ bus->phy_mask &= ~(1 << id);
+
+ irq = of_irq_to_resource(np, 0, NULL);
+ if (irq != NO_IRQ)
+ bus->irq[id] = irq;
+}
+
+static int __devinit mdio_ofgpio_probe(struct of_device *ofdev,
+const struct of_device_id *match)
+{
+ struct device_node *np = NULL;
+ struct mii_bus *new_bus;
+ struct mdio_gpio_info *bitbang;
+ int ret = -ENOMEM;
+ int i;
+
+ bitbang = kzalloc(sizeof(struct mdio_gpio_info), GFP_KERNEL);
+ if (!bitbang)
+ goto out;
+
+ bitbang->ctrl.ops = &mdio_gpio_ops;
+
+ new_bus = alloc_mdio_bitbang(&bitbang->ctrl);
+ if (!new_bus)
+ goto out_free_bitbang;
+
+ new_bus->name = "GPIO Bitbanged MII",
+
+ ret = mdio_ofgpio_
[PATCH 0/2] phylib: mdio-ofgpio ---> mdio-gpio (v2)
Changes since v1: - broken to rename patch and mdio-gpio rework --- Paulius Zaleckas (2): phylib: make mdio-gpio work without OF phylib: rename mdio-ofgpio to mdio-gpio drivers/net/phy/Kconfig |7 + drivers/net/phy/Makefile |2 drivers/net/phy/mdio-gpio.c | 308 + drivers/net/phy/mdio-ofgpio.c | 204 --- include/linux/mdio-gpio.h | 30 5 files changed, 344 insertions(+), 207 deletions(-) create mode 100644 drivers/net/phy/mdio-gpio.c delete mode 100644 drivers/net/phy/mdio-ofgpio.c create mode 100644 include/linux/mdio-gpio.h -- To unsubscribe from this list: send the line "unsubscribe linux-embedded" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] phylib: mdio-ofgpio ---> mdio-gpio
On Fri, Oct 31, 2008 at 9:53 AM, Paulius Zaleckas
<[EMAIL PROTECTED]> wrote:
> Rename mdio-ofgpio driver to mdio-gpio and make it work with
> non OpenFirmware gpio implementation.
Please respin this patch in 2 pieces; 1 patch to move the file and 1
patch to make the changes. It is difficult to review as a single
patch.
g.
>
> Aditional changes to mdio-ofgpio:
> - use gpio_request() and gpio_free()
> - place irq[] array in struct mdio_gpio_info
> - add module description, author and license
> - if NO_IRQ is not defined define it to 0
> - add note about compiling this driver as module
> - rename mdc and mdio function (were ugly names)
> - change MII to MDIO in bus name
> - add __init __exit to module (un)loading functions
> - probe fails if no phys added to the bus
> - kzalloc bitbang with sizeof(*bitbang)
>
> Laurent, please test this driver under OF.
>
> Signed-off-by: Paulius Zaleckas <[EMAIL PROTECTED]>
> Cc: Laurent Pinchart <[EMAIL PROTECTED]>
> Cc: Grant Likely <[EMAIL PROTECTED]>
> Cc: Mike Frysinger <[EMAIL PROTECTED]>
> ---
>
> drivers/net/phy/Kconfig |7 +
> drivers/net/phy/Makefile |2
> drivers/net/phy/mdio-gpio.c | 308
> +
> drivers/net/phy/mdio-ofgpio.c | 204 ---
> include/linux/mdio-gpio.h | 30
> 5 files changed, 344 insertions(+), 207 deletions(-)
> create mode 100644 drivers/net/phy/mdio-gpio.c
> delete mode 100644 drivers/net/phy/mdio-ofgpio.c
> create mode 100644 include/linux/mdio-gpio.h
>
>
> diff --git a/drivers/net/phy/Kconfig b/drivers/net/phy/Kconfig
> index d55932a..c4c5a2f 100644
> --- a/drivers/net/phy/Kconfig
> +++ b/drivers/net/phy/Kconfig
> @@ -84,10 +84,13 @@ config MDIO_BITBANG
>
> If in doubt, say N.
>
> -config MDIO_OF_GPIO
> +config MDIO_GPIO
>tristate "Support for GPIO lib-based bitbanged MDIO buses"
> - depends on MDIO_BITBANG && OF_GPIO
> + depends on MDIO_BITBANG && GENERIC_GPIO
>---help---
> Supports GPIO lib-based MDIO busses.
>
> + To compile this driver as a module, choose M here: the module
> + will be called mdio-gpio.
> +
> endif # PHYLIB
> diff --git a/drivers/net/phy/Makefile b/drivers/net/phy/Makefile
> index eee329f..9ae5d30 100644
> --- a/drivers/net/phy/Makefile
> +++ b/drivers/net/phy/Makefile
> @@ -15,4 +15,4 @@ obj-$(CONFIG_ICPLUS_PHY) += icplus.o
> obj-$(CONFIG_REALTEK_PHY) += realtek.o
> obj-$(CONFIG_FIXED_PHY)+= fixed.o
> obj-$(CONFIG_MDIO_BITBANG) += mdio-bitbang.o
> -obj-$(CONFIG_MDIO_OF_GPIO) += mdio-ofgpio.o
> +obj-$(CONFIG_MDIO_GPIO)+= mdio-gpio.o
> diff --git a/drivers/net/phy/mdio-gpio.c b/drivers/net/phy/mdio-gpio.c
> new file mode 100644
> index 000..a7b94ac
> --- /dev/null
> +++ b/drivers/net/phy/mdio-gpio.c
> @@ -0,0 +1,308 @@
> +/*
> + * GPIO based MDIO bitbang driver.
> + * Supports OpenFirmware.
> + *
> + * Copyright (c) 2008 CSE Semaphore Belgium.
> + * by Laurent Pinchart <[EMAIL PROTECTED]>
> + *
> + * Copyright (C) 2008, Paulius Zaleckas <[EMAIL PROTECTED]>
> + *
> + * Based on earlier work by
> + *
> + * Copyright (c) 2003 Intracom S.A.
> + * by Pantelis Antoniou <[EMAIL PROTECTED]>
> + *
> + * 2005 (c) MontaVista Software, Inc.
> + * Vitaly Bordug <[EMAIL PROTECTED]>
> + *
> + * This file is licensed under the terms of the GNU General Public License
> + * version 2. This program is licensed "as is" without any warranty of any
> + * kind, whether express or implied.
> + */
> +
> +#include
> +#include
> +#include
> +#include
> +#include
> +#ifdef CONFIG_OF_GPIO
> +# include
> +# include
> +#else
> +# include
> +# include
> +# include
> +#endif
> +
> +#ifndef NO_IRQ
> +#define NO_IRQ 0
> +#endif
> +
> +struct mdio_gpio_info {
> + struct mdiobb_ctrl ctrl;
> + int mdc, mdio;
> + int irq[PHY_MAX_ADDR];
> +};
> +
> +static void mdio_dir(struct mdiobb_ctrl *ctrl, int dir)
> +{
> + struct mdio_gpio_info *bitbang =
> + container_of(ctrl, struct mdio_gpio_info, ctrl);
> +
> + if (dir)
> + gpio_direction_output(bitbang->mdio, 1);
> + else
> + gpio_direction_input(bitbang->mdio);
> +}
> +
> +static int mdio_get(struct mdiobb_ctrl *ctrl)
> +{
> + struct mdio_gpio_info *bitbang =
> + container_of(ctrl, struct mdio_gpio_info, ctrl);
> +
> + return gpio_get_value(bitbang->mdio);
> +}
> +
> +static void mdio_set(struct mdiobb_ctrl *ctrl, int what)
> +{
> + struct mdio_gpio_info *bitbang =
> + container_of(ctrl, struct mdio_gpio_info, ctrl);
> +
> + gpio_set_value(bitbang->mdio, what);
> +}
> +
> +static void mdc_set(struct mdiobb_ctrl *ctrl, int what)
> +{
> + struct mdio_gpio_info *bitbang =
> + container_of(ctrl, struct mdio_gpio_info, ctrl);
> +
> + gpio_set_value(bitbang->mdc, what);
> +}
> +
> +static struct mdiobb_ops mdio_gpio_ops = {
> + .ow
[PATCH] phylib: mdio-ofgpio ---> mdio-gpio
Rename mdio-ofgpio driver to mdio-gpio and make it work with
non OpenFirmware gpio implementation.
Aditional changes to mdio-ofgpio:
- use gpio_request() and gpio_free()
- place irq[] array in struct mdio_gpio_info
- add module description, author and license
- if NO_IRQ is not defined define it to 0
- add note about compiling this driver as module
- rename mdc and mdio function (were ugly names)
- change MII to MDIO in bus name
- add __init __exit to module (un)loading functions
- probe fails if no phys added to the bus
- kzalloc bitbang with sizeof(*bitbang)
Laurent, please test this driver under OF.
Signed-off-by: Paulius Zaleckas <[EMAIL PROTECTED]>
Cc: Laurent Pinchart <[EMAIL PROTECTED]>
Cc: Grant Likely <[EMAIL PROTECTED]>
Cc: Mike Frysinger <[EMAIL PROTECTED]>
---
drivers/net/phy/Kconfig |7 +
drivers/net/phy/Makefile |2
drivers/net/phy/mdio-gpio.c | 308 +
drivers/net/phy/mdio-ofgpio.c | 204 ---
include/linux/mdio-gpio.h | 30
5 files changed, 344 insertions(+), 207 deletions(-)
create mode 100644 drivers/net/phy/mdio-gpio.c
delete mode 100644 drivers/net/phy/mdio-ofgpio.c
create mode 100644 include/linux/mdio-gpio.h
diff --git a/drivers/net/phy/Kconfig b/drivers/net/phy/Kconfig
index d55932a..c4c5a2f 100644
--- a/drivers/net/phy/Kconfig
+++ b/drivers/net/phy/Kconfig
@@ -84,10 +84,13 @@ config MDIO_BITBANG
If in doubt, say N.
-config MDIO_OF_GPIO
+config MDIO_GPIO
tristate "Support for GPIO lib-based bitbanged MDIO buses"
- depends on MDIO_BITBANG && OF_GPIO
+ depends on MDIO_BITBANG && GENERIC_GPIO
---help---
Supports GPIO lib-based MDIO busses.
+ To compile this driver as a module, choose M here: the module
+ will be called mdio-gpio.
+
endif # PHYLIB
diff --git a/drivers/net/phy/Makefile b/drivers/net/phy/Makefile
index eee329f..9ae5d30 100644
--- a/drivers/net/phy/Makefile
+++ b/drivers/net/phy/Makefile
@@ -15,4 +15,4 @@ obj-$(CONFIG_ICPLUS_PHY) += icplus.o
obj-$(CONFIG_REALTEK_PHY) += realtek.o
obj-$(CONFIG_FIXED_PHY)+= fixed.o
obj-$(CONFIG_MDIO_BITBANG) += mdio-bitbang.o
-obj-$(CONFIG_MDIO_OF_GPIO) += mdio-ofgpio.o
+obj-$(CONFIG_MDIO_GPIO)+= mdio-gpio.o
diff --git a/drivers/net/phy/mdio-gpio.c b/drivers/net/phy/mdio-gpio.c
new file mode 100644
index 000..a7b94ac
--- /dev/null
+++ b/drivers/net/phy/mdio-gpio.c
@@ -0,0 +1,308 @@
+/*
+ * GPIO based MDIO bitbang driver.
+ * Supports OpenFirmware.
+ *
+ * Copyright (c) 2008 CSE Semaphore Belgium.
+ * by Laurent Pinchart <[EMAIL PROTECTED]>
+ *
+ * Copyright (C) 2008, Paulius Zaleckas <[EMAIL PROTECTED]>
+ *
+ * Based on earlier work by
+ *
+ * Copyright (c) 2003 Intracom S.A.
+ * by Pantelis Antoniou <[EMAIL PROTECTED]>
+ *
+ * 2005 (c) MontaVista Software, Inc.
+ * Vitaly Bordug <[EMAIL PROTECTED]>
+ *
+ * This file is licensed under the terms of the GNU General Public License
+ * version 2. This program is licensed "as is" without any warranty of any
+ * kind, whether express or implied.
+ */
+
+#include
+#include
+#include
+#include
+#include
+#ifdef CONFIG_OF_GPIO
+# include
+# include
+#else
+# include
+# include
+# include
+#endif
+
+#ifndef NO_IRQ
+#define NO_IRQ 0
+#endif
+
+struct mdio_gpio_info {
+ struct mdiobb_ctrl ctrl;
+ int mdc, mdio;
+ int irq[PHY_MAX_ADDR];
+};
+
+static void mdio_dir(struct mdiobb_ctrl *ctrl, int dir)
+{
+ struct mdio_gpio_info *bitbang =
+ container_of(ctrl, struct mdio_gpio_info, ctrl);
+
+ if (dir)
+ gpio_direction_output(bitbang->mdio, 1);
+ else
+ gpio_direction_input(bitbang->mdio);
+}
+
+static int mdio_get(struct mdiobb_ctrl *ctrl)
+{
+ struct mdio_gpio_info *bitbang =
+ container_of(ctrl, struct mdio_gpio_info, ctrl);
+
+ return gpio_get_value(bitbang->mdio);
+}
+
+static void mdio_set(struct mdiobb_ctrl *ctrl, int what)
+{
+ struct mdio_gpio_info *bitbang =
+ container_of(ctrl, struct mdio_gpio_info, ctrl);
+
+ gpio_set_value(bitbang->mdio, what);
+}
+
+static void mdc_set(struct mdiobb_ctrl *ctrl, int what)
+{
+ struct mdio_gpio_info *bitbang =
+ container_of(ctrl, struct mdio_gpio_info, ctrl);
+
+ gpio_set_value(bitbang->mdc, what);
+}
+
+static struct mdiobb_ops mdio_gpio_ops = {
+ .owner = THIS_MODULE,
+ .set_mdc = mdc_set,
+ .set_mdio_dir = mdio_dir,
+ .set_mdio_data = mdio_set,
+ .get_mdio_data = mdio_get,
+};
+
+#ifdef CONFIG_OF_GPIO
+static int __devinit mdio_ofgpio_bitbang_init(struct mii_bus *bus,
+ struct device_node *np)
+{
+ struct mdio_gpio_info *bitbang = bus->priv;
+
+ bitbang->mdc = of_get_gpio(np, 0);
+ bitbang->mdio = of_get_gpio(np, 1);
+
+ if (bitbang->mdc < 0 || bitbang->mdio < 0)
+
Re: [PATCH V2 10/16] Squashfs: cache operations
Jörn Engel wrote: Only one of your problems seems to be real. Not sure if or how we can solve that one, though. Sorry don't agree. But I'm not going to argue this like a couple of old maids. I'll keep what I currently do unless Andrew Morton or someone else says it's not getting mainlined unless it's changed. Phillip -- To unsubscribe from this list: send the line "unsubscribe linux-embedded" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH V2 10/16] Squashfs: cache operations
On Fri, 31 October 2008 04:43:46 +, Phillip Lougher wrote:
>
> Simplicity and speed is extremely important. The
> squashfs_metadata_read() wrapper around the cache is designed to step
> through the metadata a structure at a time (20 or so bytes), updating
> the read position in the metadata each call, with more metadata cache
> blocks being read and decompressed as necessary. The common case where
> the metadata is already in the cache (because we're stepping through it
> 20 or so bytes at a time), is designed to be extremely fast - a spinlock
> and array search only. I recently optimised the cache to use spinlocks
> rather than mutexes and reduced the lock holding time (necessary to move
> to spinlocks), and this resulted in a 20%+ speed improvement in reading
> squashfs filesystems.
For the page cache, you can use read_cache_page() and
page_cache_release(). This does not even take a spinlock, hence
multiple readers can work in parallel. The radix tree walk will take a
similar time to your array search.
You are aware that multiple cpus will play pingpong with your spinlock?
> Given the above using an address space in the page cache will result in
> greater complexity, more memory overhead, and much slower operation.
> There's a number of reasons for this.
>
> 1. The amount and life-span of the data stored in the page cache is
> outside of Squashfs' control. As explained above it only makes sense to
> temporarily cache the last couple of metadata and fragment blocks.
> Using the page cache (if a 'large' address space is used) for these
> keeps more of them around for longer than necessary, and will
> potentially cause more worthy datablocks to be flushed on memory pressure.
I personally find it hard to guess access patterns. If I constructed a
workload just large enough that your cache is slightly too small, it
will start thrashing. Hit rates will go close to zero. And unless I
missed something, such a workload can be constructed and may even be
used in real life. With growing numbers of cores, this becomes
increasingly likely.
In other cases, an idle squashfs will still hold the 64k of unused cache
for ransom. By giving up control, you allow your cache to grow and
shrink as desired and can avoid both cases.
> 2. The address space will be caching uncompressed data, the squashfs
> references to this data are the compressed locations within the
> filesystem. There doesn't exist a one-to-one linear mapping from
> compressed location to decompressed location in the address space. This
> means a lookup table still needs to be used to store the mapping from
> compressed location to decompressed location in the address space. Now
> this lookup table (however implemented) is itself at least as complex as
> my current cache implementation.
You are currently using physical offset of the medium to address blocks
in the cache, which are 64bit. Page cache may use 32bit to address
pages. And since blocks can be larger than pages, you effectively need
some bits extra. This is indeed a problem.
> 3. Once the locations of the decompressed pages in the address space
> have been found, they'll need to be looked up in the page cache, and
> this has to be done for every 4K page. With the default fragment size
> of 128 KiB this means 32 separate lookups. Somewhat slower than one
> spinlock and array search per 128 KiB block in the squashfs cache
> implementation.
Above you claimed the complete cache to be just 64k in size. How can
you access 128k blocks that way? One of the numbers appears wrong.
Ignoring that, you don't need to take either the spinlock or do a lookup
in a fast path. If you currently have this code:
for (some condition) {
err = squashfs_read_metadata(address, ...);
}
You can transform it to:
void *handle = squashfs_get_metadata(address, ...);
for (some condition) {
err = squashfs_read_metadata(handle, address, ...);
}
squashfs_put_metadata(handle);
With the handle you can keep a reference count to your cached object.
squashfs_read_metadata() only has to call squashfs_cache_get() or
squashfs_cache_put() when moving across object boundaries. In the
common case it simply returns data from the object referenced through
the handle.
This might be a worthwhile optimization independently of whether you use
the page cache or not.
> Comments, especially those of the form "you've got this completely
> wrong, and you can use the page cache like this, which will be simpler
> and faster than your current implementation" welcome :) I'm not adverse
> to using the page cache, but I can't see how it will be simpler or
> faster than the current implementation.
Only one of your problems seems to be real. Not sure if or how we can
solve that one, though.
Jörn
--
"Security vulnerabilities are here to stay."
-- Scott Culp, Manager of the Microsoft Security Response Center
