Re: [PATCH 2/7] soc/fman: Add the FMan FLIB
On Wed, 2015-03-04 at 23:45 -0600, Emil Medve wrote: > From: Igal Liberman > > Signed-off-by: Igal Liberman > --- > drivers/soc/Kconfig |1 + > drivers/soc/Makefile |1 + > drivers/soc/fsl/Kconfig |1 + > drivers/soc/fsl/Makefile |1 + > drivers/soc/fsl/fman/Kconfig |7 + > drivers/soc/fsl/fman/Makefile | 13 + > drivers/soc/fsl/fman/fman.c | 1396 > + > 7 files changed, 1420 insertions(+) > create mode 100644 drivers/soc/fsl/Kconfig > create mode 100644 drivers/soc/fsl/Makefile > create mode 100644 drivers/soc/fsl/fman/Kconfig > create mode 100644 drivers/soc/fsl/fman/Makefile > create mode 100644 drivers/soc/fsl/fman/fman.c > > diff --git a/drivers/soc/Kconfig b/drivers/soc/Kconfig > index 76d6bd4..674a6e6 100644 > --- a/drivers/soc/Kconfig > +++ b/drivers/soc/Kconfig > @@ -1,5 +1,6 @@ > menu "SOC (System On Chip) specific Drivers" > > +source "drivers/soc/fsl/Kconfig" > source "drivers/soc/qcom/Kconfig" > source "drivers/soc/ti/Kconfig" > source "drivers/soc/versatile/Kconfig" > diff --git a/drivers/soc/Makefile b/drivers/soc/Makefile > index 063113d..42836ee 100644 > --- a/drivers/soc/Makefile > +++ b/drivers/soc/Makefile > @@ -2,6 +2,7 @@ > # Makefile for the Linux Kernel SOC specific device drivers. > # > > +obj-$(CONFIG_FSL_SOC)+= fsl/ > obj-$(CONFIG_ARCH_QCOM) += qcom/ > obj-$(CONFIG_ARCH_TEGRA) += tegra/ > obj-$(CONFIG_SOC_TI) += ti/ > diff --git a/drivers/soc/fsl/Kconfig b/drivers/soc/fsl/Kconfig > new file mode 100644 > index 000..38c08ae > --- /dev/null > +++ b/drivers/soc/fsl/Kconfig > @@ -0,0 +1 @@ > +source "drivers/soc/fsl/fman/Kconfig" > diff --git a/drivers/soc/fsl/Makefile b/drivers/soc/fsl/Makefile > new file mode 100644 > index 000..97d715c > --- /dev/null > +++ b/drivers/soc/fsl/Makefile > @@ -0,0 +1 @@ > +obj-$(CONFIG_FSL_FMAN) += fman/ > diff --git a/drivers/soc/fsl/fman/Kconfig b/drivers/soc/fsl/fman/Kconfig > new file mode 100644 > index 000..e5009a9 > --- /dev/null > +++ b/drivers/soc/fsl/fman/Kconfig > @@ -0,0 +1,7 @@ > +menuconfig FSL_FMAN > + tristate "Freescale DPAA Frame Manager" > + depends on FSL_SOC || COMPILE_TEST > + default n > + help > + Freescale Data-Path Acceleration Architecture Frame Manager > + (FMan) support As this doesn't appear to be a complete driver, why is it user-selectable? Likewise for the other flib patches. Why are you adding the flibs in a big batch, rather than bundling the code with the driver patches that use it? Is there a minimal core of fman functionality (complete functionality, not flibs) that you could start with, rather than trying to get the entire thing reviewed at once? In other words, break it up vertically, not horizontally. Note that the lack of caller context is especially harmful given that the functions are not documented. > diff --git a/drivers/soc/fsl/fman/Makefile b/drivers/soc/fsl/fman/Makefile > new file mode 100644 > index 000..d7fbecb > --- /dev/null > +++ b/drivers/soc/fsl/fman/Makefile > @@ -0,0 +1,13 @@ > +ccflags-y += -DVERSION=\"\" > + > +ifeq ($(CONFIG_FSL_FMAN),y) > + > +FMAN = $(srctree)/drivers/soc/fsl/fman > + > +ccflags-y += -I$(FMAN)/flib > + > +obj-$(CONFIG_FSL_FMAN) += fsl_fman.o > + > +fsl_fman-objs:= fman.o > + > +endif You won't even get into this makefile if CONFIG_FSL_FMAN=n... Did you really mean to exclude this stuff when CONFIG_FSL_FMAN=m? > +int fman_reset_mac(struct fman_fpm_regs __iomem *fpm_rg, uint8_t mac_id) > +{ > + uint32_t msk, timeout = 100; > + > + /* Get the relevant bit mask */ > + switch (mac_id) { > + case (0): > + msk = FPM_RSTC_MAC0_RESET; > + break; > + case (1): > + msk = FPM_RSTC_MAC1_RESET; > + break; > + case (2): > + msk = FPM_RSTC_MAC2_RESET; > + break; > + case (3): > + msk = FPM_RSTC_MAC3_RESET; > + break; > + case (4): > + msk = FPM_RSTC_MAC4_RESET; > + break; > + case (5): > + msk = FPM_RSTC_MAC5_RESET; > + break; > + case (6): > + msk = FPM_RSTC_MAC6_RESET; > + break; > + case (7): > + msk = FPM_RSTC_MAC7_RESET; > + break; > + case (8): > + msk = FPM_RSTC_MAC8_RESET; > + break; > + case (9): > + msk = FPM_RSTC_MAC9_RESET; > + break; > + default: > + return -EINVAL; > + } Without seeing the caller, I can't judge whether there's a good reason for passing this in by number rather than passing in FPM_RSTC_MACn_RESET directly and avoiding the switch. > + /* reset */ > + iowrite32be(msk, _rg->fm_rstc); > + while ((ioread32be(_rg->fm_rstc) & msk) && --timeout) > + usleep_range(10, 11); > + > +
Re: [PATCH 2/7] soc/fman: Add the FMan FLIB
On Wed, 2015-03-04 at 23:45 -0600, Emil Medve wrote: From: Igal Liberman igal.liber...@freescale.com Signed-off-by: Igal Liberman igal.liber...@freescale.com --- drivers/soc/Kconfig |1 + drivers/soc/Makefile |1 + drivers/soc/fsl/Kconfig |1 + drivers/soc/fsl/Makefile |1 + drivers/soc/fsl/fman/Kconfig |7 + drivers/soc/fsl/fman/Makefile | 13 + drivers/soc/fsl/fman/fman.c | 1396 + 7 files changed, 1420 insertions(+) create mode 100644 drivers/soc/fsl/Kconfig create mode 100644 drivers/soc/fsl/Makefile create mode 100644 drivers/soc/fsl/fman/Kconfig create mode 100644 drivers/soc/fsl/fman/Makefile create mode 100644 drivers/soc/fsl/fman/fman.c diff --git a/drivers/soc/Kconfig b/drivers/soc/Kconfig index 76d6bd4..674a6e6 100644 --- a/drivers/soc/Kconfig +++ b/drivers/soc/Kconfig @@ -1,5 +1,6 @@ menu SOC (System On Chip) specific Drivers +source drivers/soc/fsl/Kconfig source drivers/soc/qcom/Kconfig source drivers/soc/ti/Kconfig source drivers/soc/versatile/Kconfig diff --git a/drivers/soc/Makefile b/drivers/soc/Makefile index 063113d..42836ee 100644 --- a/drivers/soc/Makefile +++ b/drivers/soc/Makefile @@ -2,6 +2,7 @@ # Makefile for the Linux Kernel SOC specific device drivers. # +obj-$(CONFIG_FSL_SOC)+= fsl/ obj-$(CONFIG_ARCH_QCOM) += qcom/ obj-$(CONFIG_ARCH_TEGRA) += tegra/ obj-$(CONFIG_SOC_TI) += ti/ diff --git a/drivers/soc/fsl/Kconfig b/drivers/soc/fsl/Kconfig new file mode 100644 index 000..38c08ae --- /dev/null +++ b/drivers/soc/fsl/Kconfig @@ -0,0 +1 @@ +source drivers/soc/fsl/fman/Kconfig diff --git a/drivers/soc/fsl/Makefile b/drivers/soc/fsl/Makefile new file mode 100644 index 000..97d715c --- /dev/null +++ b/drivers/soc/fsl/Makefile @@ -0,0 +1 @@ +obj-$(CONFIG_FSL_FMAN) += fman/ diff --git a/drivers/soc/fsl/fman/Kconfig b/drivers/soc/fsl/fman/Kconfig new file mode 100644 index 000..e5009a9 --- /dev/null +++ b/drivers/soc/fsl/fman/Kconfig @@ -0,0 +1,7 @@ +menuconfig FSL_FMAN + tristate Freescale DPAA Frame Manager + depends on FSL_SOC || COMPILE_TEST + default n + help + Freescale Data-Path Acceleration Architecture Frame Manager + (FMan) support As this doesn't appear to be a complete driver, why is it user-selectable? Likewise for the other flib patches. Why are you adding the flibs in a big batch, rather than bundling the code with the driver patches that use it? Is there a minimal core of fman functionality (complete functionality, not flibs) that you could start with, rather than trying to get the entire thing reviewed at once? In other words, break it up vertically, not horizontally. Note that the lack of caller context is especially harmful given that the functions are not documented. diff --git a/drivers/soc/fsl/fman/Makefile b/drivers/soc/fsl/fman/Makefile new file mode 100644 index 000..d7fbecb --- /dev/null +++ b/drivers/soc/fsl/fman/Makefile @@ -0,0 +1,13 @@ +ccflags-y += -DVERSION=\\ + +ifeq ($(CONFIG_FSL_FMAN),y) + +FMAN = $(srctree)/drivers/soc/fsl/fman + +ccflags-y += -I$(FMAN)/flib + +obj-$(CONFIG_FSL_FMAN) += fsl_fman.o + +fsl_fman-objs:= fman.o + +endif You won't even get into this makefile if CONFIG_FSL_FMAN=n... Did you really mean to exclude this stuff when CONFIG_FSL_FMAN=m? +int fman_reset_mac(struct fman_fpm_regs __iomem *fpm_rg, uint8_t mac_id) +{ + uint32_t msk, timeout = 100; + + /* Get the relevant bit mask */ + switch (mac_id) { + case (0): + msk = FPM_RSTC_MAC0_RESET; + break; + case (1): + msk = FPM_RSTC_MAC1_RESET; + break; + case (2): + msk = FPM_RSTC_MAC2_RESET; + break; + case (3): + msk = FPM_RSTC_MAC3_RESET; + break; + case (4): + msk = FPM_RSTC_MAC4_RESET; + break; + case (5): + msk = FPM_RSTC_MAC5_RESET; + break; + case (6): + msk = FPM_RSTC_MAC6_RESET; + break; + case (7): + msk = FPM_RSTC_MAC7_RESET; + break; + case (8): + msk = FPM_RSTC_MAC8_RESET; + break; + case (9): + msk = FPM_RSTC_MAC9_RESET; + break; + default: + return -EINVAL; + } Without seeing the caller, I can't judge whether there's a good reason for passing this in by number rather than passing in FPM_RSTC_MACn_RESET directly and avoiding the switch. + /* reset */ + iowrite32be(msk, fpm_rg-fm_rstc); + while ((ioread32be(fpm_rg-fm_rstc) msk) --timeout) + usleep_range(10, 11); + + if (!timeout) + return -EBUSY; + return 0; +} -EBUSY generally means
[PATCH 2/7] soc/fman: Add the FMan FLIB
From: Igal Liberman Signed-off-by: Igal Liberman --- drivers/soc/Kconfig |1 + drivers/soc/Makefile |1 + drivers/soc/fsl/Kconfig |1 + drivers/soc/fsl/Makefile |1 + drivers/soc/fsl/fman/Kconfig |7 + drivers/soc/fsl/fman/Makefile | 13 + drivers/soc/fsl/fman/fman.c | 1396 + 7 files changed, 1420 insertions(+) create mode 100644 drivers/soc/fsl/Kconfig create mode 100644 drivers/soc/fsl/Makefile create mode 100644 drivers/soc/fsl/fman/Kconfig create mode 100644 drivers/soc/fsl/fman/Makefile create mode 100644 drivers/soc/fsl/fman/fman.c diff --git a/drivers/soc/Kconfig b/drivers/soc/Kconfig index 76d6bd4..674a6e6 100644 --- a/drivers/soc/Kconfig +++ b/drivers/soc/Kconfig @@ -1,5 +1,6 @@ menu "SOC (System On Chip) specific Drivers" +source "drivers/soc/fsl/Kconfig" source "drivers/soc/qcom/Kconfig" source "drivers/soc/ti/Kconfig" source "drivers/soc/versatile/Kconfig" diff --git a/drivers/soc/Makefile b/drivers/soc/Makefile index 063113d..42836ee 100644 --- a/drivers/soc/Makefile +++ b/drivers/soc/Makefile @@ -2,6 +2,7 @@ # Makefile for the Linux Kernel SOC specific device drivers. # +obj-$(CONFIG_FSL_SOC) += fsl/ obj-$(CONFIG_ARCH_QCOM)+= qcom/ obj-$(CONFIG_ARCH_TEGRA) += tegra/ obj-$(CONFIG_SOC_TI) += ti/ diff --git a/drivers/soc/fsl/Kconfig b/drivers/soc/fsl/Kconfig new file mode 100644 index 000..38c08ae --- /dev/null +++ b/drivers/soc/fsl/Kconfig @@ -0,0 +1 @@ +source "drivers/soc/fsl/fman/Kconfig" diff --git a/drivers/soc/fsl/Makefile b/drivers/soc/fsl/Makefile new file mode 100644 index 000..97d715c --- /dev/null +++ b/drivers/soc/fsl/Makefile @@ -0,0 +1 @@ +obj-$(CONFIG_FSL_FMAN) += fman/ diff --git a/drivers/soc/fsl/fman/Kconfig b/drivers/soc/fsl/fman/Kconfig new file mode 100644 index 000..e5009a9 --- /dev/null +++ b/drivers/soc/fsl/fman/Kconfig @@ -0,0 +1,7 @@ +menuconfig FSL_FMAN + tristate "Freescale DPAA Frame Manager" + depends on FSL_SOC || COMPILE_TEST + default n + help + Freescale Data-Path Acceleration Architecture Frame Manager + (FMan) support diff --git a/drivers/soc/fsl/fman/Makefile b/drivers/soc/fsl/fman/Makefile new file mode 100644 index 000..d7fbecb --- /dev/null +++ b/drivers/soc/fsl/fman/Makefile @@ -0,0 +1,13 @@ +ccflags-y += -DVERSION=\"\" + +ifeq ($(CONFIG_FSL_FMAN),y) + +FMAN = $(srctree)/drivers/soc/fsl/fman + +ccflags-y += -I$(FMAN)/flib + +obj-$(CONFIG_FSL_FMAN) += fsl_fman.o + +fsl_fman-objs := fman.o + +endif diff --git a/drivers/soc/fsl/fman/fman.c b/drivers/soc/fsl/fman/fman.c new file mode 100644 index 000..dbf2cd3 --- /dev/null +++ b/drivers/soc/fsl/fman/fman.c @@ -0,0 +1,1396 @@ +/* + * Copyright 2008-2015 Freescale Semiconductor Inc. + * + * Redistribution and use in source and binary forms, with or without + * modification, are permitted provided that the following conditions are met: + * * Redistributions of source code must retain the above copyright + * notice, this list of conditions and the following disclaimer. + * * Redistributions in binary form must reproduce the above copyright + * notice, this list of conditions and the following disclaimer in the + * documentation and/or other materials provided with the distribution. + * * Neither the name of Freescale Semiconductor nor the + * names of its contributors may be used to endorse or promote products + * derived from this software without specific prior written permission. + * + * + * ALTERNATIVELY, this software may be distributed under the terms of the + * GNU General Public License ("GPL") as published by the Free Software + * Foundation, either version 2 of that License or (at your option) any + * later version. + * + * THIS SOFTWARE IS PROVIDED BY Freescale Semiconductor ``AS IS'' AND ANY + * EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE IMPLIED + * WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE ARE + * DISCLAIMED. IN NO EVENT SHALL Freescale Semiconductor BE LIABLE FOR ANY + * DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES + * (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; + * LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND + * ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT + * (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE OF THIS + * SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. + */ + +#include "fsl_fman.h" + +uint32_t fman_get_bmi_err_event(struct fman_bmi_regs __iomem *bmi_rg) +{ + uint32_t event, mask, force; + + event = ioread32be(_rg->fmbm_ievr); + mask = ioread32be(_rg->fmbm_ier); + event &= mask; + /* clear the forced events */ + force = ioread32be(_rg->fmbm_ifr); +
[PATCH 2/7] soc/fman: Add the FMan FLIB
From: Igal Liberman igal.liber...@freescale.com Signed-off-by: Igal Liberman igal.liber...@freescale.com --- drivers/soc/Kconfig |1 + drivers/soc/Makefile |1 + drivers/soc/fsl/Kconfig |1 + drivers/soc/fsl/Makefile |1 + drivers/soc/fsl/fman/Kconfig |7 + drivers/soc/fsl/fman/Makefile | 13 + drivers/soc/fsl/fman/fman.c | 1396 + 7 files changed, 1420 insertions(+) create mode 100644 drivers/soc/fsl/Kconfig create mode 100644 drivers/soc/fsl/Makefile create mode 100644 drivers/soc/fsl/fman/Kconfig create mode 100644 drivers/soc/fsl/fman/Makefile create mode 100644 drivers/soc/fsl/fman/fman.c diff --git a/drivers/soc/Kconfig b/drivers/soc/Kconfig index 76d6bd4..674a6e6 100644 --- a/drivers/soc/Kconfig +++ b/drivers/soc/Kconfig @@ -1,5 +1,6 @@ menu SOC (System On Chip) specific Drivers +source drivers/soc/fsl/Kconfig source drivers/soc/qcom/Kconfig source drivers/soc/ti/Kconfig source drivers/soc/versatile/Kconfig diff --git a/drivers/soc/Makefile b/drivers/soc/Makefile index 063113d..42836ee 100644 --- a/drivers/soc/Makefile +++ b/drivers/soc/Makefile @@ -2,6 +2,7 @@ # Makefile for the Linux Kernel SOC specific device drivers. # +obj-$(CONFIG_FSL_SOC) += fsl/ obj-$(CONFIG_ARCH_QCOM)+= qcom/ obj-$(CONFIG_ARCH_TEGRA) += tegra/ obj-$(CONFIG_SOC_TI) += ti/ diff --git a/drivers/soc/fsl/Kconfig b/drivers/soc/fsl/Kconfig new file mode 100644 index 000..38c08ae --- /dev/null +++ b/drivers/soc/fsl/Kconfig @@ -0,0 +1 @@ +source drivers/soc/fsl/fman/Kconfig diff --git a/drivers/soc/fsl/Makefile b/drivers/soc/fsl/Makefile new file mode 100644 index 000..97d715c --- /dev/null +++ b/drivers/soc/fsl/Makefile @@ -0,0 +1 @@ +obj-$(CONFIG_FSL_FMAN) += fman/ diff --git a/drivers/soc/fsl/fman/Kconfig b/drivers/soc/fsl/fman/Kconfig new file mode 100644 index 000..e5009a9 --- /dev/null +++ b/drivers/soc/fsl/fman/Kconfig @@ -0,0 +1,7 @@ +menuconfig FSL_FMAN + tristate Freescale DPAA Frame Manager + depends on FSL_SOC || COMPILE_TEST + default n + help + Freescale Data-Path Acceleration Architecture Frame Manager + (FMan) support diff --git a/drivers/soc/fsl/fman/Makefile b/drivers/soc/fsl/fman/Makefile new file mode 100644 index 000..d7fbecb --- /dev/null +++ b/drivers/soc/fsl/fman/Makefile @@ -0,0 +1,13 @@ +ccflags-y += -DVERSION=\\ + +ifeq ($(CONFIG_FSL_FMAN),y) + +FMAN = $(srctree)/drivers/soc/fsl/fman + +ccflags-y += -I$(FMAN)/flib + +obj-$(CONFIG_FSL_FMAN) += fsl_fman.o + +fsl_fman-objs := fman.o + +endif diff --git a/drivers/soc/fsl/fman/fman.c b/drivers/soc/fsl/fman/fman.c new file mode 100644 index 000..dbf2cd3 --- /dev/null +++ b/drivers/soc/fsl/fman/fman.c @@ -0,0 +1,1396 @@ +/* + * Copyright 2008-2015 Freescale Semiconductor Inc. + * + * Redistribution and use in source and binary forms, with or without + * modification, are permitted provided that the following conditions are met: + * * Redistributions of source code must retain the above copyright + * notice, this list of conditions and the following disclaimer. + * * Redistributions in binary form must reproduce the above copyright + * notice, this list of conditions and the following disclaimer in the + * documentation and/or other materials provided with the distribution. + * * Neither the name of Freescale Semiconductor nor the + * names of its contributors may be used to endorse or promote products + * derived from this software without specific prior written permission. + * + * + * ALTERNATIVELY, this software may be distributed under the terms of the + * GNU General Public License (GPL) as published by the Free Software + * Foundation, either version 2 of that License or (at your option) any + * later version. + * + * THIS SOFTWARE IS PROVIDED BY Freescale Semiconductor ``AS IS'' AND ANY + * EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE IMPLIED + * WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE ARE + * DISCLAIMED. IN NO EVENT SHALL Freescale Semiconductor BE LIABLE FOR ANY + * DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES + * (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; + * LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND + * ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT + * (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE OF THIS + * SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. + */ + +#include fsl_fman.h + +uint32_t fman_get_bmi_err_event(struct fman_bmi_regs __iomem *bmi_rg) +{ + uint32_t event, mask, force; + + event = ioread32be(bmi_rg-fmbm_ievr); + mask = ioread32be(bmi_rg-fmbm_ier); + event = mask; + /* clear the forced events */ + force =