Re: [PATCH] profile: setup_profiling_timer() is moslty not implemented

2022-07-25 Thread Ben Dooks

On 25/07/2022 20:39, Andrew Morton wrote:

On Thu, 21 Jul 2022 20:55:09 +0100 Ben Dooks  wrote:


The setup_profiling_timer() is mostly un-implemented by many
architectures. In many places it isn't guarded by CONFIG_PROFILE
which is needed for it to be used. Make it a weak symbol in
kernel/profile.c and remove the 'return -EINVAL' implementations
from the kenrel.

There are a couple of architectures which do return 0 from
the setup_profiling_timer() function but they don't seem to
do anything else with it. To keep the /proc compatibility for
now, leave these for a future update or removal.

On ARM, this fixes the following sparse warning:
arch/arm/kernel/smp.c:793:5: warning: symbol 'setup_profiling_timer' was not 
declared. Should it be static?


I'll grab this.

We have had some problems with weak functions lately.  See

https://lore.kernel.org/all/87ee0q7b92@email.froward.int.ebiederm.org/T/#u

Hopefully that was a rare corner case.


Great, thanks.

--
Ben Dooks   http://www.codethink.co.uk/
Senior Engineer Codethink - Providing Genius

https://www.codethink.co.uk/privacy.html


Re: [PATCH V7 13/20] riscv: compat: process: Add UXL_32 support in start_thread

2022-03-11 Thread Ben Dooks

On 11/03/2022 02:38, Guo Ren wrote:

Hi Arnd,

On Mon, Feb 28, 2022 at 12:30 AM  wrote:


From: Guo Ren 

If the current task is in COMPAT mode, set SR_UXL_32 in status for
returning userspace. We need CONFIG _COMPAT to prevent compiling
errors with rv32 defconfig.

Signed-off-by: Guo Ren 
Signed-off-by: Guo Ren 
Cc: Arnd Bergmann 
Cc: Palmer Dabbelt 
---
  arch/riscv/kernel/process.c | 5 +
  1 file changed, 5 insertions(+)

diff --git a/arch/riscv/kernel/process.c b/arch/riscv/kernel/process.c
index 03ac3aa611f5..54787ca9806a 100644
--- a/arch/riscv/kernel/process.c
+++ b/arch/riscv/kernel/process.c
@@ -97,6 +97,11 @@ void start_thread(struct pt_regs *regs, unsigned long pc,
 }
 regs->epc = pc;
 regs->sp = sp;
+

FIxup:

+ #ifdef CONFIG_COMPAT

+   if (is_compat_task())
+   regs->status = (regs->status & ~SR_UXL) | SR_UXL_32;
+   else
+   regs->status = (regs->status & ~SR_UXL) | SR_UXL_64;

+ #endif

We still need "#ifdef CONFIG_COMPAT" here, because for rv32 we can't
set SR_UXL at all. SR_UXL is BIT[32, 33].


would an if (IS_ENABLED(CONFIG_COMPAT)) { } around the lot be better
than an #ifdef here?


  }

  void flush_thread(void)
--
2.25.1







--
Ben Dooks   http://www.codethink.co.uk/
Senior Engineer Codethink - Providing Genius

https://www.codethink.co.uk/privacy.html


Re: [PATCH] Raise the minimum GCC version to 5.2

2021-05-04 Thread Ben Dooks

On 02/05/2021 03:41, Joe Perches wrote:

On Sat, 2021-05-01 at 17:52 +0200, Miguel Ojeda wrote:

On Sat, May 1, 2021 at 5:17 PM Masahiro Yamada  wrote:


More cleanups will be possible as follow-up patches, but this one must
be agreed and applied to the mainline first.


+1 This will allow me to remove the __has_attribute hack in
include/linux/compiler_attributes.h.


Why not raise the minimum gcc compiler version even higher?

https://gcc.gnu.org/releases.html


Some of us are a bit stuck as either customer refuses to upgrade
their build infrastructure or has paid for some old but safety
blessed version of gcc. These often lag years behind the recent
gcc releases :(


--
Ben Dooks   http://www.codethink.co.uk/
Senior Engineer Codethink - Providing Genius

https://www.codethink.co.uk/privacy.html


Re: [PATCH 2/2] gpio: gpiolib: set gpiochip_remove retval to void

2014-06-08 Thread Ben Dooks
On Fri, May 30, 2014 at 08:16:59PM +0200, Lars-Peter Clausen wrote:
> On 05/30/2014 07:33 PM, David Daney wrote:
> >On 05/30/2014 04:39 AM, Geert Uytterhoeven wrote:
> >>On Fri, May 30, 2014 at 1:30 PM, abdoulaye berthe 
> >>wrote:
> >>>--- a/drivers/gpio/gpiolib.c
> >>>+++ b/drivers/gpio/gpiolib.c
> >>>@@ -1263,10 +1263,9 @@ static void gpiochip_irqchip_remove(struct
> >>>gpio_chip *gpiochip);
> >>>   *
> >>>   * A gpio_chip with any GPIOs still requested may not be removed.
> >>>   */
> >>>-int gpiochip_remove(struct gpio_chip *chip)
> >>>+void gpiochip_remove(struct gpio_chip *chip)
> >>>  {
> >>> unsigned long   flags;
> >>>-   int status = 0;
> >>> unsignedid;
> >>>
> >>> acpi_gpiochip_remove(chip);
> >>>@@ -1278,24 +1277,15 @@ int gpiochip_remove(struct gpio_chip *chip)
> >>> of_gpiochip_remove(chip);
> >>>
> >>> for (id = 0; id < chip->ngpio; id++) {
> >>>-   if (test_bit(FLAG_REQUESTED, &chip->desc[id].flags)) {
> >>>-   status = -EBUSY;
> >>>-   break;
> >>>-   }
> >>>-   }
> >>>-   if (status == 0) {
> >>>-   for (id = 0; id < chip->ngpio; id++)
> >>>-   chip->desc[id].chip = NULL;
> >>>-
> >>>-   list_del(&chip->list);
> >>>+   if (test_bit(FLAG_REQUESTED, &chip->desc[id].flags))
> >>>+   panic("gpio: removing gpiochip with gpios still
> >>>requested\n");
> >>
> >>panic?
> >
> >NACK to the patch for this reason.  The strongest thing you should do here
> >is WARN.
> >
> >That said, I am not sure why we need this whole patch set in the first place.
> 
> Well, what currently happens when you remove a device that is a
> provider of a gpio_chip which is still in use, is that the kernel
> crashes. Probably with a rather cryptic error message. So this patch
> doesn't really change the behavior, but makes it more explicit what
> is actually wrong. And even if you replace the panic() by a WARN()
> it will again just crash slightly later.
> 
> This is a design flaw in the GPIO subsystem that needs to be fixed.

Surely then the best way is to error out to the module unload and
stop the driver being unloaded?

-- 
Ben Dooks, b...@fluff.org, http://www.fluff.org/ben/

Large Hadron Colada: A large Pina Colada that makes the universe disappear.

___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev

Re: [PATCH] i2c-mpc: Add support for 64bit system

2011-03-15 Thread Ben Dooks
On Tue, Mar 15, 2011 at 11:02:43AM -0500, Kumar Gala wrote:
> Currently i2c-mpc supports 32bit system only, this modification makes it
> supported on both 32-bit and 64-bit systems.  The P5020 is the first
> 64-bit PPC system with the i2c-mpc controller.
> 
> Based in patch from Xulei 
> 
> Signed-off-by: Kumar Gala 

ok, will add to -next

> ---
>  drivers/i2c/busses/Kconfig |2 +-
>  1 files changed, 1 insertions(+), 1 deletions(-)
> 
> diff --git a/drivers/i2c/busses/Kconfig b/drivers/i2c/busses/Kconfig
> index 113505a..107d5c2 100644
> --- a/drivers/i2c/busses/Kconfig
> +++ b/drivers/i2c/busses/Kconfig
> @@ -433,7 +433,7 @@ config I2C_IXP2000
>  
>  config I2C_MPC
>   tristate "MPC107/824x/85xx/512x/52xx/83xx/86xx"
> - depends on PPC32
> + depends on PPC
>   help
> If you say yes to this option, support will be included for the
> built-in I2C interface on the MPC107, Tsi107, MPC512x, MPC52xx,
> -- 
> 1.7.2.3
> 

-- 
Ben Dooks, b...@fluff.org, http://www.fluff.org/ben/

Large Hadron Colada: A large Pina Colada that makes the universe disappear.

___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev


Re: [PATCH] I2C: Add support for 64bit system.

2010-12-20 Thread Ben Dooks
On Mon, Dec 20, 2010 at 03:37:34PM +0800, Xulei wrote:
> Currently I2C_MPC supports 32bit system only, then this
> modification makes it support 32bit and 64bit system both.
> 
> Signed-off-by: Xulei 

This been build or run tested?

> ---
>  drivers/i2c/busses/Kconfig |2 +-
>  1 files changed, 1 insertions(+), 1 deletions(-)
> 
> diff --git a/drivers/i2c/busses/Kconfig b/drivers/i2c/busses/Kconfig
> index 9c6170c..3392f4b 100644
> --- a/drivers/i2c/busses/Kconfig
> +++ b/drivers/i2c/busses/Kconfig
> @@ -422,7 +422,7 @@ config I2C_IXP2000
>  
>  config I2C_MPC
>   tristate "MPC107/824x/85xx/512x/52xx/83xx/86xx"
> - depends on PPC32
> + depends on PPC32 || PPC64
>   help
> If you say yes to this option, support will be included for the
> built-in I2C interface on the MPC107, Tsi107, MPC512x, MPC52xx,
> -- 
> 1.7.0.4
> 
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-i2c" in
> the body of a message to majord...@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

-- 
Ben Dooks, b...@fluff.org, http://www.fluff.org/ben/

Large Hadron Colada: A large Pina Colada that makes the universe disappear.

___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev


Re: [PATCH (Option 1)] of/i2c: fix module load order issue caused by of_i2c.c

2010-09-28 Thread Ben Dooks
On Fri, Sep 24, 2010 at 04:14:53PM -0600, Grant Likely wrote:
> Commit 959e85f7, "i2c: add OF-style registration and binding" caused a
> module dependency loop where of_i2c.c calls functions in i2c-core, and
> i2c-core calls of_i2c_register_devices() in of_i2c.  This means that
> when i2c support is built as a module when CONFIG_OF is set, then
> neither i2c_core nor of_i2c are able to be loaded.
> 
> This patch fixes the problem by moving the of_i2c_register_devices()
> function into the body of i2c_core and renaming it to
> i2c_scan_of_devices (of_i2c_register_devices is analogous to the
> existing i2c_scan_static_board_info function and so should be named
> similarly).  This function isn't called by any code outside of
> i2c_core, and it must always be present when CONFIG_OF is selected, so
> it makes sense to locate it there.  When CONFIG_OF is not selected,
> of_i2c_register_devices() becomes a no-op.

I sort of go with this one.

-- 
Ben

Q:  What's a light-year?
A:  One-third less calories than a regular year.

___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev


Re: [PATCH (Option 1)] of/i2c: fix module load order issue caused by of_i2c.c

2010-09-28 Thread Ben Dooks
On Fri, Sep 24, 2010 at 04:14:53PM -0600, Grant Likely wrote:
> Commit 959e85f7, "i2c: add OF-style registration and binding" caused a
> module dependency loop where of_i2c.c calls functions in i2c-core, and
> i2c-core calls of_i2c_register_devices() in of_i2c.  This means that
> when i2c support is built as a module when CONFIG_OF is set, then
> neither i2c_core nor of_i2c are able to be loaded.
> 
> This patch fixes the problem by moving the of_i2c_register_devices()
> function into the body of i2c_core and renaming it to
> i2c_scan_of_devices (of_i2c_register_devices is analogous to the
> existing i2c_scan_static_board_info function and so should be named
> similarly).  This function isn't called by any code outside of
> i2c_core, and it must always be present when CONFIG_OF is selected, so
> it makes sense to locate it there.  When CONFIG_OF is not selected,
> of_i2c_register_devices() becomes a no-op.
> 
> Signed-off-by: Grant Likely 
> ---
>  drivers/i2c/i2c-core.c |   61 
> ++--
>  drivers/of/of_i2c.c|   57 -
>  include/linux/of_i2c.h |7 --
>  3 files changed, 59 insertions(+), 66 deletions(-)
> 
> diff --git a/drivers/i2c/i2c-core.c b/drivers/i2c/i2c-core.c
> index 6649176..64a261b 100644
> --- a/drivers/i2c/i2c-core.c
> +++ b/drivers/i2c/i2c-core.c
> @@ -32,8 +32,8 @@
>  #include 
>  #include 
>  #include 
> -#include 
>  #include 
> +#include 
>  #include 
>  #include 
>  #include 
> @@ -818,6 +818,63 @@ static void i2c_scan_static_board_info(struct 
> i2c_adapter *adapter)
>   up_read(&__i2c_board_lock);
>  }
>  
> +#ifdef CONFIG_OF
> +void i2c_scan_of_devices(struct i2c_adapter *adap)
> +{
> + void *result;
> + struct device_node *node;
> +
> + /* Only register child devices if the adapter has a node pointer set */
> + if (!adap->dev.of_node)
> + return;
> +
> + for_each_child_of_node(adap->dev.of_node, node) {
> + struct i2c_board_info info = {};
> + struct dev_archdata dev_ad = {};
> + const __be32 *addr;
> + int len;
> +
> + dev_dbg(&adap->dev, "of_i2c: register %s\n", node->full_name);
> + if (of_modalias_node(node, info.type, sizeof(info.type)) < 0) {
> + dev_err(&adap->dev, "of_i2c: modalias failure on %s\n",
> + node->full_name);
> + continue;
> + }
> +
> + addr = of_get_property(node, "reg", &len);
> + if (!addr || (len < sizeof(int))) {
> + dev_err(&adap->dev, "of_i2c: invalid reg on %s\n",
> + node->full_name);
> + continue;
> + }
> +
> + info.addr = be32_to_cpup(addr);
> + if (info.addr > (1 << 10) - 1) {
> + dev_err(&adap->dev, "of_i2c: invalid addr=%x on %s\n",
> + info.addr, node->full_name);
> + continue;
> + }
> +
> + info.irq = irq_of_parse_and_map(node, 0);
> + info.of_node = of_node_get(node);
> + info.archdata = &dev_ad;
> +
> + request_module("%s", info.type);
> +
> + result = i2c_new_device(adap, &info);
> + if (result == NULL) {
> + dev_err(&adap->dev, "of_i2c: Failure registering %s\n",
> + node->full_name);
> + of_node_put(node);
> + irq_dispose_mapping(info.irq);
> + continue;
> + }
> + }
> +}
> +#else
> +static inline void i2c_scan_of_devices(struct i2c_adapter *adap) { }
> +#endif

Is there any advantage to just making the definition in the header
file a static inline and removing the #else part of this?

-- 
Ben

Q:  What's a light-year?
A:  One-third less calories than a regular year.

___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev


Re: Request review of device tree documentation

2010-06-14 Thread Ben Dooks
On Sun, Jun 13, 2010 at 11:36:57PM -0600, Grant Likely wrote:
> On Sun, Jun 13, 2010 at 2:29 AM, Benjamin Herrenschmidt
>  wrote:
> > On Sat, 2010-06-12 at 20:45 -1000, Mitch Bradley wrote:
> >
> >> Either fought or embraced.  To the extent that it is possible to focus
> >> solely on Linux and ARM, one could image doing a good HAL.
> >
> > That will come with a huge amount of comunity resistance sadly, but I
> > can imagine distros liking it.
> >
> > In general, I much prefer having all the necessary native drivers in the
> > kernel, and the device-tree to provide the right representation, and
> > avoid trying to abstract "methods" via a HAL. It's the Linux philosophy
> > as much as possible (even when defeated by ACPI).
> >
> > But if we're going to be forced by vendors into HALs, we can also make
> > sure that whatever they come up with is half reasonable :-)
> 
> I think there is more to be concerned about regarding binary blobs
> than HALs.  Many of the new SoCs require closed binaries to use all
> the hardware right now (graphics cores in particular).
> 
> Board vendors seem to be taking the plunge and modifying the kernel
> rather than trying to create a HAL for driving board specific
> features.

In my view HALs are a bad idea, they constrain you to one calling method
and make it difficult to evolve support in the kernel. I belive it is
part of the reason that we've always tried to avoid a standardised
kernel driver interface.

-- 
Ben

Q:  What's a light-year?
A:  One-third less calories than a regular year.

___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev


Re: [PATCH 33/37] sound/soc: use .dev.of_node instead of .node in struct of_device

2010-04-05 Thread Ben Dooks
On Thu, Mar 11, 2010 at 11:06:50AM -0700, Grant Likely wrote:
> .node is being removed
> 
> Signed-off-by: Grant Likely 
> ---
> 
>  sound/soc/fsl/mpc8610_hpcd.c |2 +-
>  1 files changed, 1 insertions(+), 1 deletions(-)
> 
> diff --git a/sound/soc/fsl/mpc8610_hpcd.c b/sound/soc/fsl/mpc8610_hpcd.c
> index ef67d1c..d7e1b9a 100644
> --- a/sound/soc/fsl/mpc8610_hpcd.c
> +++ b/sound/soc/fsl/mpc8610_hpcd.c
> @@ -202,7 +202,7 @@ static struct snd_soc_ops mpc8610_hpcd_ops = {
>  static int mpc8610_hpcd_probe(struct of_device *ofdev,
>   const struct of_device_id *match)
>  {
> - struct device_node *np = ofdev->node;
> + struct device_node *np = ofdev->dev.of_node;
>   struct device_node *codec_np = NULL;
>   struct device_node *guts_np = NULL;
>   struct device_node *dma_np = NULL;

This looks like one case where an inline function would have been a
help.


-- 
Ben (b...@fluff.org, http://www.fluff.org/)

  'a smiley only costs 4 bytes'
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev


Re: [PATCH v2 1/3] i2c-mpc: use __devinit[data] for initialization functions and data

2010-01-27 Thread Ben Dooks
On Tue, Jan 26, 2010 at 07:44:10PM +0100, Wolfgang Grandegger wrote:
> Ben Dooks wrote:
> > On Mon, Jan 25, 2010 at 09:55:04PM +0100, Wolfgang Grandegger wrote:
> >> From: Wolfgang Grandegger 
> >>
> >> "__devinit[data]" has not yet been used for all initialization functions
> >> and data. To avoid truncating lines, the struct mpc_i2c_match_data has
> >> been renamed to mpc_i2c_data, which is even the better name.
> >>
> >> Signed-off-by: Wolfgang Grandegger 
> >> ---
> >>  drivers/i2c/busses/i2c-mpc.c |   99 
> >> +++--
> >>  1 files changed, 46 insertions(+), 53 deletions(-)
> >>
> >> diff --git a/drivers/i2c/busses/i2c-mpc.c b/drivers/i2c/busses/i2c-mpc.c
> >> index f627001..2cb864e 100644
> >> --- a/drivers/i2c/busses/i2c-mpc.c
> >> +++ b/drivers/i2c/busses/i2c-mpc.c
> >> @@ -66,7 +66,7 @@ struct mpc_i2c_divider {
> >>u16 fdr;/* including dfsrr */
> >>  };
> >>  
> >> -struct mpc_i2c_match_data {
> >> +struct mpc_i2c_data {
> >>void (*setclock)(struct device_node *node,
> >> struct mpc_i2c *i2c,
> >> u32 clock, u32 prescaler);
> >> @@ -165,7 +165,7 @@ static int i2c_wait(struct mpc_i2c *i2c, unsigned 
> >> timeout, int writing)
> >>  }
> >>  
> >>  #ifdef CONFIG_PPC_MPC52xx
> >> -static const struct mpc_i2c_divider mpc_i2c_dividers_52xx[] = {
> >> +static const struct __devinitdata mpc_i2c_divider mpc_i2c_dividers_52xx[] 
> >> = {
> >>{20, 0x20}, {22, 0x21}, {24, 0x22}, {26, 0x23},
> >>{28, 0x24}, {30, 0x01}, {32, 0x25}, {34, 0x02},
> >>{36, 0x26}, {40, 0x27}, {44, 0x04}, {48, 0x28},
> >> @@ -186,7 +186,8 @@ static const struct mpc_i2c_divider 
> >> mpc_i2c_dividers_52xx[] = {
> >>{10240, 0x9d}, {12288, 0x9e}, {15360, 0x9f}
> >>  };
> >>  
> >> -int mpc_i2c_get_fdr_52xx(struct device_node *node, u32 clock, int 
> >> prescaler)
> >> +static int __devinit mpc_i2c_get_fdr_52xx(struct device_node *node, u32 
> >> clock,
> >> +int prescaler)
> >>  {
> >>const struct mpc_i2c_divider *div = NULL;
> >>unsigned int pvr = mfspr(SPRN_PVR);
> >> @@ -215,9 +216,9 @@ int mpc_i2c_get_fdr_52xx(struct device_node *node, u32 
> >> clock, int prescaler)
> >>return div ? (int)div->fdr : -EINVAL;
> >>  }
> >>  
> >> -static void mpc_i2c_setclock_52xx(struct device_node *node,
> >> -struct mpc_i2c *i2c,
> >> -u32 clock, u32 prescaler)
> >> +static void __devinit mpc_i2c_setclock_52xx(struct device_node *node,
> >> +  struct mpc_i2c *i2c,
> >> +  u32 clock, u32 prescaler)
> >>  {
> >>int ret, fdr;
> >>  
> >> @@ -230,15 +231,15 @@ static void mpc_i2c_setclock_52xx(struct device_node 
> >> *node,
> >>dev_info(i2c->dev, "clock %d Hz (fdr=%d)\n", clock, fdr);
> >>  }
> >>  #else /* !CONFIG_PPC_MPC52xx */
> >> -static void mpc_i2c_setclock_52xx(struct device_node *node,
> >> -struct mpc_i2c *i2c,
> >> -u32 clock, u32 prescaler)
> >> +static void __devinit mpc_i2c_setclock_52xx(struct device_node *node,
> >> +  struct mpc_i2c *i2c,
> >> +  u32 clock, u32 prescaler)
> >>  {
> >>  }
> >>  #endif /* CONFIG_PPC_MPC52xx*/
> >>  
> >>  #ifdef CONFIG_FSL_SOC
> >> -static const struct mpc_i2c_divider mpc_i2c_dividers_8xxx[] = {
> >> +static const struct __devinitdata mpc_i2c_divider mpc_i2c_dividers_8xxx[] 
> >> = {
> >>{160, 0x0120}, {192, 0x0121}, {224, 0x0122}, {256, 0x0123},
> >>{288, 0x0100}, {320, 0x0101}, {352, 0x0601}, {384, 0x0102},
> >>{416, 0x0602}, {448, 0x0126}, {480, 0x0103}, {512, 0x0127},
> >> @@ -258,7 +259,7 @@ static const struct mpc_i2c_divider 
> >> mpc_i2c_dividers_8xxx[] = {
> >>{49152, 0x011e}, {61440, 0x011f}
> >>  };
> >>  
> >> -u32 mpc_i2c_get_sec_cfg_8xxx(void)
> >> +static u32 __devinit mpc_i2c_get_sec_cfg_8xxx(void)
> >>  {
> >>struct device_node *node = NULL;
> >>u32 __iomem *reg;
> 

Re: [PATCH v2 1/3] i2c-mpc: use __devinit[data] for initialization functions and data

2010-01-26 Thread Ben Dooks
On Mon, Jan 25, 2010 at 09:55:04PM +0100, Wolfgang Grandegger wrote:
> From: Wolfgang Grandegger 
> 
> "__devinit[data]" has not yet been used for all initialization functions
> and data. To avoid truncating lines, the struct mpc_i2c_match_data has
> been renamed to mpc_i2c_data, which is even the better name.
> 
> Signed-off-by: Wolfgang Grandegger 
> ---
>  drivers/i2c/busses/i2c-mpc.c |   99 +++--
>  1 files changed, 46 insertions(+), 53 deletions(-)
> 
> diff --git a/drivers/i2c/busses/i2c-mpc.c b/drivers/i2c/busses/i2c-mpc.c
> index f627001..2cb864e 100644
> --- a/drivers/i2c/busses/i2c-mpc.c
> +++ b/drivers/i2c/busses/i2c-mpc.c
> @@ -66,7 +66,7 @@ struct mpc_i2c_divider {
>   u16 fdr;/* including dfsrr */
>  };
>  
> -struct mpc_i2c_match_data {
> +struct mpc_i2c_data {
>   void (*setclock)(struct device_node *node,
>struct mpc_i2c *i2c,
>u32 clock, u32 prescaler);
> @@ -165,7 +165,7 @@ static int i2c_wait(struct mpc_i2c *i2c, unsigned 
> timeout, int writing)
>  }
>  
>  #ifdef CONFIG_PPC_MPC52xx
> -static const struct mpc_i2c_divider mpc_i2c_dividers_52xx[] = {
> +static const struct __devinitdata mpc_i2c_divider mpc_i2c_dividers_52xx[] = {
>   {20, 0x20}, {22, 0x21}, {24, 0x22}, {26, 0x23},
>   {28, 0x24}, {30, 0x01}, {32, 0x25}, {34, 0x02},
>   {36, 0x26}, {40, 0x27}, {44, 0x04}, {48, 0x28},
> @@ -186,7 +186,8 @@ static const struct mpc_i2c_divider 
> mpc_i2c_dividers_52xx[] = {
>   {10240, 0x9d}, {12288, 0x9e}, {15360, 0x9f}
>  };
>  
> -int mpc_i2c_get_fdr_52xx(struct device_node *node, u32 clock, int prescaler)
> +static int __devinit mpc_i2c_get_fdr_52xx(struct device_node *node, u32 
> clock,
> +   int prescaler)
>  {
>   const struct mpc_i2c_divider *div = NULL;
>   unsigned int pvr = mfspr(SPRN_PVR);
> @@ -215,9 +216,9 @@ int mpc_i2c_get_fdr_52xx(struct device_node *node, u32 
> clock, int prescaler)
>   return div ? (int)div->fdr : -EINVAL;
>  }
>  
> -static void mpc_i2c_setclock_52xx(struct device_node *node,
> -   struct mpc_i2c *i2c,
> -   u32 clock, u32 prescaler)
> +static void __devinit mpc_i2c_setclock_52xx(struct device_node *node,
> + struct mpc_i2c *i2c,
> + u32 clock, u32 prescaler)
>  {
>   int ret, fdr;
>  
> @@ -230,15 +231,15 @@ static void mpc_i2c_setclock_52xx(struct device_node 
> *node,
>   dev_info(i2c->dev, "clock %d Hz (fdr=%d)\n", clock, fdr);
>  }
>  #else /* !CONFIG_PPC_MPC52xx */
> -static void mpc_i2c_setclock_52xx(struct device_node *node,
> -   struct mpc_i2c *i2c,
> -   u32 clock, u32 prescaler)
> +static void __devinit mpc_i2c_setclock_52xx(struct device_node *node,
> + struct mpc_i2c *i2c,
> + u32 clock, u32 prescaler)
>  {
>  }
>  #endif /* CONFIG_PPC_MPC52xx*/
>  
>  #ifdef CONFIG_FSL_SOC
> -static const struct mpc_i2c_divider mpc_i2c_dividers_8xxx[] = {
> +static const struct __devinitdata mpc_i2c_divider mpc_i2c_dividers_8xxx[] = {
>   {160, 0x0120}, {192, 0x0121}, {224, 0x0122}, {256, 0x0123},
>   {288, 0x0100}, {320, 0x0101}, {352, 0x0601}, {384, 0x0102},
>   {416, 0x0602}, {448, 0x0126}, {480, 0x0103}, {512, 0x0127},
> @@ -258,7 +259,7 @@ static const struct mpc_i2c_divider 
> mpc_i2c_dividers_8xxx[] = {
>   {49152, 0x011e}, {61440, 0x011f}
>  };
>  
> -u32 mpc_i2c_get_sec_cfg_8xxx(void)
> +static u32 __devinit mpc_i2c_get_sec_cfg_8xxx(void)
>  {
>   struct device_node *node = NULL;
>   u32 __iomem *reg;
> @@ -287,7 +288,8 @@ u32 mpc_i2c_get_sec_cfg_8xxx(void)
>   return val;
>  }
>  
> -int mpc_i2c_get_fdr_8xxx(struct device_node *node, u32 clock, u32 prescaler)
> +static int __devinit mpc_i2c_get_fdr_8xxx(struct device_node *node, u32 
> clock,
> +   u32 prescaler)
>  {
>   const struct mpc_i2c_divider *div = NULL;
>   u32 divider;
> @@ -320,9 +322,9 @@ int mpc_i2c_get_fdr_8xxx(struct device_node *node, u32 
> clock, u32 prescaler)
>   return div ? (int)div->fdr : -EINVAL;
>  }
>  
> -static void mpc_i2c_setclock_8xxx(struct device_node *node,
> -   struct mpc_i2c *i2c,
> -   u32 clock, u32 prescaler)
> +static void __devinit mpc_i2c_setclock_8xxx(struct device_node *node,
> + struct mpc_i2c *i2c,
> + u32 clock, u32 prescaler)
>  {
>   int ret, fdr;
>  
> @@ -338,9 +340,9 @@ static void mpc_i2c_setclock_8xxx(struct device_node 
> *node,
>  }
>  
>  #else /* !CONFIG_FSL_SOC */
> -static void mpc_i2c_setclock_8xxx(struct device_node *node,
> -   struct mpc_i2c *i2c,

Re: [PATCH 2/3] i2c-mpc: add support for the MPC512x processors from Freescale

2010-01-26 Thread Ben Dooks
On Mon, Jan 25, 2010 at 04:15:09PM +0100, Wolfram Sang wrote:
> > >>  
> > >> -static void __devinit mpc_i2c_setclock_52xx(struct device_node *node,
> > >> -struct mpc_i2c *i2c,
> > >> -u32 clock, u32 prescaler)
> > >> +static void __devinit mpc_i2c_setup_52xx(struct device_node *node,
> > >> + struct mpc_i2c *i2c,
> > >> + u32 clock, u32 prescaler)
> > >>  {
> > >>  int ret, fdr;
> > >>  
> > >> +if (clock == -1) {
> > > 
> > > Could we use 0 for 'no_clock'? This would make the above statement simply
> > 
> > "0" is already used to maintain backward compatibility setting a safe
> > divider.
> 
> Ah, now I see:
> 
> 'clock == -1' means 'preserve clocks' (and is checked here in 
> mpc_i2c_setup_52xx())
> 'clock ==  0' means 'safe divider' (and is checked in mpc_i2c_get_fdr_52xx())

hmm, sounds like a job for a  #define or similar.
 
> This is not a beauty ;)
> 
> What about adding a flags variable to the setup-functions?
> 
> Regards,
> 
>Wolfram
> 
> -- 
> Pengutronix e.K.   | Wolfram Sang|
> Industrial Linux Solutions | http://www.pengutronix.de/  |



-- 
Ben (b...@fluff.org, http://www.fluff.org/)

  'a smiley only costs 4 bytes'
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev


Re: [PATCH/RFC 1/2] 5200: improve i2c bus error recovery

2010-01-24 Thread Ben Dooks
On Fri, Jan 22, 2010 at 09:17:55PM +0100, Albrecht Dreß wrote:
> Improve the recovery of the MPC5200B's I2C bus from errors like bus
> hangs.

This is very sparse comapred to the large comment below the --- line,
maybe some more description should be living up here.

Is thios a candidate for an -rc or should it be left to merge window?
 
> Signed-off-by: Albrecht Dreß 
> 
> ---
> 
> This patch introduces several improvements to the MPC5200B's I2C driver
> as to improve the recovery from error conditions I encountered when
> testing a custom board with several I2C devices attached (eeprom, io
> expander, rtc, sensors).  The error conditions included cases where the
> bus if logic of one slave apparently went south, blocking the bus
> completely.
> 
> My fixes include:
> 1. make the bus timeout configurable in fsl_i2c_probe(); the default of
>one second is *way* too long for my use case;
> 2. if a timeout condition occurs in mpc_xfer(), mpc_i2c_fixup() the bus
>if *any* of the CF, BB and RXAK flags in the MSR is 1.  I actually
>saw different combinations with hangs, not only all three set;
> 3. improve the fixup procedure by calculating the timing needed from the
>real (configured) bus clock, calculated in mpc_i2c_setclock_52xx().
>Furthermore, I issue 9 instead of one cycle, as I experienced cases
>where the single one is not enough (found this tip in a forum).  As a
>side effect, the new scheme needs only 81us @375kHz bus clock instead
>of 150us.  I recorded waveforms for 18.4kHz, 85.9kHz and 375kHz, all
>looking fine, which I can provide if anyone is interested.
> 
> Open questions:
> - is the approach correct at all, in particular the interpretation of
>   the flags (#2)?
> - could this code also be used on non-5200 processors?
> 
> --- linux-2.6.32-orig/drivers/i2c/busses/i2c-mpc.c2009-12-03 
> 04:51:21.0 +0100
> +++ linux-2.6.32/drivers/i2c/busses/i2c-mpc.c 2010-01-22 16:05:13.0 
> +0100
> @@ -59,6 +59,7 @@ struct mpc_i2c {
>   wait_queue_head_t queue;
>   struct i2c_adapter adap;
>   int irq;
> + u32 real_clk;
>  };
>  
>  struct mpc_i2c_divider {
> @@ -97,16 +98,32 @@ static irqreturn_t mpc_i2c_isr(int irq, 
>   */
>  static void mpc_i2c_fixup(struct mpc_i2c *i2c)
>  {
> - writeccr(i2c, 0);
> - udelay(30);
> - writeccr(i2c, CCR_MEN);
> - udelay(30);
> - writeccr(i2c, CCR_MSTA | CCR_MTX);
> - udelay(30);
> - writeccr(i2c, CCR_MSTA | CCR_MTX | CCR_MEN);
> - udelay(30);
> - writeccr(i2c, CCR_MEN);
> - udelay(30);
> + if (i2c->real_clk == 0) {
> + writeccr(i2c, 0);
> + udelay(30);
> + writeccr(i2c, CCR_MEN);
> + udelay(30);
> + writeccr(i2c, CCR_MSTA | CCR_MTX);
> + udelay(30);
> + writeccr(i2c, CCR_MSTA | CCR_MTX | CCR_MEN);
> + udelay(30);
> + writeccr(i2c, CCR_MEN);
> + udelay(30);
> + } else {
> + int k;
> + u32 delay_val = 100 / i2c->real_clk + 1;
> +
> + if (delay_val < 2)
> + delay_val = 2;
> +
> + for (k = 9; k; k--) {
> + writeccr(i2c, 0);
> + writeccr(i2c, CCR_MSTA | CCR_MTX | CCR_MEN);
> + udelay(delay_val);
> + writeccr(i2c, CCR_MEN);
> + udelay(delay_val << 1);
> + }
> + }
>  }
>  
>  static int i2c_wait(struct mpc_i2c *i2c, unsigned timeout, int writing)
> @@ -186,15 +203,18 @@ static const struct mpc_i2c_divider mpc_
>   {10240, 0x9d}, {12288, 0x9e}, {15360, 0x9f}
>  };
>  
> -int mpc_i2c_get_fdr_52xx(struct device_node *node, u32 clock, int prescaler)
> +int mpc_i2c_get_fdr_52xx(struct device_node *node, u32 clock, int prescaler,
> +  u32 *real_clk)
>  {
>   const struct mpc_i2c_divider *div = NULL;
>   unsigned int pvr = mfspr(SPRN_PVR);
>   u32 divider;
>   int i;
>  
> - if (!clock)
> + if (!clock) {
> + *real_clk = 0;
>   return -EINVAL;
> + }
>  
>   /* Determine divider value */
>   divider = mpc5xxx_get_bus_frequency(node) / clock;
> @@ -212,7 +232,8 @@ int mpc_i2c_get_fdr_52xx(struct device_n
>   break;
>   }
>  
> - return div ? (int)div->fdr : -EINVAL;
> + *real_clk = mpc5xxx_get_bus_frequency(node) / div->divider;
> + return (int)div->fdr;
>  }
>  
>  static void mpc_i2c_setclock_52xx(struct device_node *node,
> @@ -221,13 +242,14 @@ static void mpc_i2c_setclock_52xx(struct
>  {
>   int ret, fdr;
>  
> - ret = mpc_i2c_get_fdr_52xx(node, clock, prescaler);
> + ret = mpc_i2c_get_fdr_52xx(node, clock, prescaler, &i2c->real_clk);
>   fdr = (ret >= 0) ? ret : 0x3f; /* backward compatibility */
>  
>   writeb(fdr & 0xff, i2c->base + MPC_I2C_FDR);
>  
>   if (ret >= 0)
> - dev_info(i2c->dev, "clock %d Hz (fdr=

Re: [RFC,PATCH 1/7 v2] Add a common struct clk

2010-01-12 Thread Ben Dooks
On Tue, Jan 12, 2010 at 09:01:49AM +, Russell King - ARM Linux wrote:
> On Tue, Jan 12, 2010 at 09:48:44AM +0100, Francesco VIRLINZI wrote:
> > Hi Jeremy
> > In November I already sent a proposal on
> >  a generic linux clk framework.
> > On that I would suggest:
> >
> >>
> >> +struct clk {
> >> +  const struct clk_operations *ops;
> >>
> >spinlock_t lock;
> >const char *name;
> >int id;
> 
> Name and ID are totally pointless, unless you insist on using the clk
> API in the wrong way (like S3C does.)

I do intend to change the clock lookup on the Samsung series, but we're
currently in the process of trying to do a whole pile of work on not
only adding new SoCs but also cleaning up the existing support and making
a whole pile of code common to all the Samsung SoC family.

I never got the time to go throughly through Francesco's clock framework
but it seemed rathe rcomplicated for our current requirements and also
had a whole pile of style problems that made it really difficult to read.

-- 
Ben

Q:  What's a light-year?
A:  One-third less calories than a regular year.

___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev


Re: [RFC,PATCH 1/7 v2] Add a common struct clk

2010-01-12 Thread Ben Dooks
On Tue, Jan 12, 2010 at 09:01:49AM +, Russell King - ARM Linux wrote:
> On Tue, Jan 12, 2010 at 09:48:44AM +0100, Francesco VIRLINZI wrote:
> > Hi Jeremy
> > In November I already sent a proposal on
> >  a generic linux clk framework.
> > On that I would suggest:
> >
> >>
> >> +struct clk {
> >> +  const struct clk_operations *ops;
> >>
> >spinlock_t lock;
> >const char *name;
> >int id;
> 
> Name and ID are totally pointless, unless you insist on using the clk
> API in the wrong way (like S3C does.)

I assume by this you mean that a number of the s3c drives use clk_get with
their platform device and a name? if so, I agree this needs to get sorted
out.

-- 
Ben

Q:  What's a light-year?
A:  One-third less calories than a regular year.

___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev


Re: [RFC,PATCH 1/7 v2] Add a common struct clk

2010-01-12 Thread Ben Dooks
On Tue, Jan 12, 2010 at 05:58:31PM +1100, Jeremy Kerr wrote:
> We currently have 21 definitions of struct clk in the ARM architecture,
> each defined on a per-platform basis. This makes it difficult to define
> platform- (or architecture-) independent clock sources without making
> assumptions about struct clk.
> 
> This change is an effort to unify struct clk where possible, by defining
> a common struct clk, containing a set of clock operations. Different
> clock implementations can set their own operations, and have a standard
> interface for generic code. The callback interface is exposed to the
> kernel proper, while the clock implementations only need to be seen by
> the platform internals.
> 
> This allows us to share clock code among platforms, and makes it
> possible to dynamically create clock devices in platform-independent
> code.
> 
> Platforms can enable the generic struct clock through
> CONFIG_USE_COMMON_STRUCT_CLK.
> 
> The common clock definitions are based on a development patch from Ben
> Herrenschmidt .
> 
> Signed-off-by: Jeremy Kerr 
> 
> ---
>  arch/Kconfig|3 
>  include/linux/clk.h |  134 +++-
>  2 files changed, 110 insertions(+), 27 deletions(-)
> 
> diff --git a/arch/Kconfig b/arch/Kconfig
> index 9d055b4..cefc026 100644
> --- a/arch/Kconfig
> +++ b/arch/Kconfig
> @@ -140,4 +140,7 @@ config HAVE_HW_BREAKPOINT
>  config HAVE_USER_RETURN_NOTIFIER
>   bool
>  
> +config USE_COMMON_STRUCT_CLK
> + bool
> +
>  source "kernel/gcov/Kconfig"
> diff --git a/include/linux/clk.h b/include/linux/clk.h
> index 1d37f42..1a6199e 100644
> --- a/include/linux/clk.h
> +++ b/include/linux/clk.h
> @@ -11,36 +11,101 @@
>  #ifndef __LINUX_CLK_H
>  #define __LINUX_CLK_H
>  
> -struct device;
> +#include 
>  
> -/*
> - * The base API.
> +#ifdef CONFIG_USE_COMMON_STRUCT_CLK
> +
> +/* If we're using the common struct clk, we define the base clk object here,
> + * which will be 'subclassed' by device-specific implementations. For 
> example:
> + *
> + *  struct clk_foo {
> + *  struct clk;
> + *  [device specific fields]
> + *  };
> + *
> + * We define the common clock API through a set of static inlines that call 
> the
> + * corresponding clk_operations. The API is exactly the same as that 
> documented
> + * in the !CONFIG_USE_COMMON_STRUCT_CLK case.
>   */
>  
> +struct clk {
> + const struct clk_operations *ops;
> +};
> +
> +struct clk_operations {
> +   int (*enable)(struct clk *);

Personaly I'd leave the enable/disable as one call which takes
(struct clk *, bool on). these code paths tend to be pretty much the
same read/modify/write with only the modify changing depending on the
on parameter.

> +   void(*disable)(struct clk *);
> +   unsigned long   (*get_rate)(struct clk *);
> +   void(*put)(struct clk *);
> +   long(*round_rate)(struct clk *, unsigned long);
> +   int (*set_rate)(struct clk *, unsigned long);
> +   int (*set_parent)(struct clk *, struct clk *);
> +   struct clk* (*get_parent)(struct clk *);
> +};
> +
> +static inline int clk_enable(struct clk *clk)
> +{
> + if (clk->ops->enable)
> + return clk->ops->enable(clk);
> + return 0;
> +}
> +
> +static inline void clk_disable(struct clk *clk)
> +{
> + if (clk->ops->disable)
> + clk->ops->disable(clk);
> +}
> +
> +static inline unsigned long clk_get_rate(struct clk *clk)
> +{
> + if (clk->ops->get_rate)
> + return clk->ops->get_rate(clk);
> + return 0;
> +}
> +
> +static inline void clk_put(struct clk *clk)
> +{
> + if (clk->ops->put)
> + clk->ops->put(clk);
> +}
> +
> +static inline long clk_round_rate(struct clk *clk, unsigned long rate)
> +{
> + if (clk->ops->round_rate)
> + return clk->ops->round_rate(clk, rate);
> + return -ENOSYS;
> +}
> +
> +static inline int clk_set_rate(struct clk *clk, unsigned long rate)
> +{
> + if (clk->ops->set_rate)
> + return clk->ops->set_rate(clk, rate);
> + return -ENOSYS;
> +}
> +
> +static inline int clk_set_parent(struct clk *clk, struct clk *parent)
> +{
> + if (clk->ops->set_parent)
> + return clk->ops->set_parent(clk, parent);
> + return -ENOSYS;
> +}
> +
> +static inline struct clk *clk_get_parent(struct clk *clk)
> +{
> + if (clk->ops->get_parent)
> + return clk->ops->get_parent(clk);
> + return ERR_PTR(-ENOSYS);
> +}

In the Samsung impelemtnations, the clk keeps a parent pointer and
we just return that as the esult of the clk_get_parent nless the clock
wants to override it.

> +
> +#else /* !CONFIG_USE_COMMON_STRUCT_CLK */
>  
>  /*
> - * struct clk - an machine class defined object / cookie.
> + * Global clock object, actual structure is declared per-machine
>   */
>  struct clk;
>  
>  /**
> - * clk_get - lookup and obtain a reference to a clock producer.
> - * @de

Re: [PATCH v2] i2c-mpc: generate START condition after STOP caused by read i2c_msg

2009-12-03 Thread Ben Dooks
On Thu, Dec 03, 2009 at 04:09:57PM +0100, Michael Lawnick wrote:
> Ben Dooks said the following:
> > On Tue, May 26, 2009 at 01:30:21PM +0200, Esben Haabendal wrote:
> >> On Tue, May 19, 2009 at 7:22 AM, Esben Haabendal  
> >> wrote:
> >> > This fixes MAL (arbitration lost) bug caused by illegal use of
> >> > RSTA (repeated START) after STOP condition generated after last byte
> >> > of reads. With this patch, it is possible to do an i2c_transfer() with
> >> > additional i2c_msg's following the I2C_M_RD messages.
> >> >
> >> > It still needs to be resolved if it is possible to fix this issue
> >> > by removing the STOP condition after reads in a robust way.
> >> >
> >> > Signed-off-by: Esben Haabendal 
> >> > ---
> >> > ?drivers/i2c/busses/i2c-mpc.c | ? ?9 +++--
> >> > ?1 files changed, 7 insertions(+), 2 deletions(-)
> >> 
> >> Any blockers to get this accepted?
> > 
> > It would be nice to get an ack from someone who can actually test
> > the driver before getting this merged.
> >  
> What is the state of this patch?
> Shouldn't we attack the problem on a more general way by inventing a
> Flag I2C_M_RESTART (or better I2C_M_NO_RESTART for backward compatibility)?
> This way the client driver is able to decide what it needs. If we do the
> choice within adapter, chance is about 50% to be wrong.

The documentation for 'struct i2c_msg' already says the STOP should only
be generated for the last message of the transfer. If STOP is being
generated for a message that isn't the last in the transfer than this
is incorrect behaviour.

Unless otherwise indicated, I'll put the patch in the series that I'll
send to Linus early next week.

-- 
Ben

Q:  What's a light-year?
A:  One-third less calories than a regular year.

___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev


Re: [PATCH 1/7] spi: Add support for device table matching

2009-07-29 Thread Ben Dooks
On Wed, Jul 29, 2009 at 09:04:57PM +0400, Anton Vorontsov wrote:
> With this patch spi drivers can use standard spi_driver.id_table and
> MODULE_DEVICE_TABLE() mechanisms to bind against the devices. Just
> like we do with I2C drivers.
> 
> This is useful when a single driver supports several variants of
> devices but it is not possible to detect them in run-time (like
> non-JEDEC chips probing in drivers/mtd/devices/m25p80.c), and
> when platform_data usage is overkill.
> 
> This patch also makes life a lot easier on OpenFirmware platforms,
> since with OF we extensively use proper device IDs in modaliases.
> 
> Signed-off-by: Anton Vorontsov 
> ---
>  drivers/spi/spi.c   |   26 +-
>  include/linux/mod_devicetable.h |   13 +
>  include/linux/spi/spi.h |   10 --
>  scripts/mod/file2alias.c|   13 +
>  4 files changed, 59 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/spi/spi.c b/drivers/spi/spi.c
> index 70845cc..1431bf2 100644
> --- a/drivers/spi/spi.c
> +++ b/drivers/spi/spi.c
> @@ -59,9 +59,24 @@ static struct device_attribute spi_dev_attrs[] = {
>   * and the sysfs version makes coldplug work too.
>   */
>  
> +static const struct spi_device_id *spi_match_id(const struct spi_device_id 
> *id,
> + const struct spi_device *sdev)
> +{
> + while (id->name[0]) {
> + if (!strcmp(sdev->modalias, id->name))
> + return id;
> + id++;
> + }
> + return NULL;
> +}
> +
>  static int spi_match_device(struct device *dev, struct device_driver *drv)
>  {
>   const struct spi_device *spi = to_spi_device(dev);
> + const struct spi_driver *sdrv = to_spi_driver(drv);
> +
> + if (sdrv->id_table)
> + return !!spi_match_id(sdrv->id_table, spi);
>  
>   return strcmp(spi->modalias, drv->name) == 0;
>  }
> @@ -121,6 +136,13 @@ struct bus_type spi_bus_type = {
>  };
>  EXPORT_SYMBOL_GPL(spi_bus_type);
>  
> +static int spi_drv_probe_id(struct device *dev)
> +{
> + const struct spi_driver *sdrv = to_spi_driver(dev->driver);
> + struct spi_device   *sdev = to_spi_device(dev);
> +
> + return sdrv->probe_id(sdev, spi_match_id(sdrv->id_table, sdev));
> +}
>  
>  static int spi_drv_probe(struct device *dev)
>  {
> @@ -151,7 +173,9 @@ static void spi_drv_shutdown(struct device *dev)
>  int spi_register_driver(struct spi_driver *sdrv)
>  {
>   sdrv->driver.bus = &spi_bus_type;
> - if (sdrv->probe)
> + if (sdrv->probe_id)
> + sdrv->driver.probe = spi_drv_probe_id;
> + else if (sdrv->probe)
>   sdrv->driver.probe = spi_drv_probe;
>   if (sdrv->remove)
>   sdrv->driver.remove = spi_drv_remove;
> diff --git a/include/linux/mod_devicetable.h b/include/linux/mod_devicetable.h
> index 1bf5900..9660dca 100644
> --- a/include/linux/mod_devicetable.h
> +++ b/include/linux/mod_devicetable.h
> @@ -399,6 +399,19 @@ struct i2c_device_id {
>   __attribute__((aligned(sizeof(kernel_ulong_t;
>  };
>  
> +/* spi */
> +
> +#define SPI_NAME_SIZE20
> +
> +struct spi_device_id {
> + char name[SPI_NAME_SIZE];
> +#ifdef __KERNEL__
> + void *data;
> +#else
> + kernel_ulong_t data;
> +#endif
> +};
> +
>  /* dmi */
>  enum dmi_field {
>   DMI_NONE,
> diff --git a/include/linux/spi/spi.h b/include/linux/spi/spi.h
> index c47c4b4..c8d92a1 100644
> --- a/include/linux/spi/spi.h
> +++ b/include/linux/spi/spi.h
> @@ -20,6 +20,7 @@
>  #define __LINUX_SPI_H
>  
>  #include 
> +#include 
>  
>  /*
>   * INTERFACES between SPI master-side drivers and SPI infrastructure.
> @@ -86,7 +87,7 @@ struct spi_device {
>   int irq;
>   void*controller_state;
>   void*controller_data;
> - charmodalias[32];
> + charmodalias[SPI_NAME_SIZE];
>  
>   /*
>* likely need more hooks for more protocol options affecting how
> @@ -145,6 +146,8 @@ struct spi_message;
>  
>  /**
>   * struct spi_driver - Host side "protocol" driver
> + * @id_table: List of SPI devices supported by this driver
> + * @probe_id: Binds this driver to the spi device via id_table matching.
>   * @probe: Binds this driver to the spi device.  Drivers can verify
>   *   that the device is actually present, and may need to configure
>   *   characteristics (such as bits_per_word) which weren't needed for
> @@ -170,6 +173,9 @@ struct spi_message;
>   * MMC, RTC, filesystem character device nodes, and hardware monitoring.
>   */
>  struct spi_driver {
> + const struct spi_device_id *id_table;
> + int (*probe_id)(struct spi_device *spi,
> + const struct spi_device_id *id);

how about leaving it at just probe and have either a call or a field
in the device that you can look at to se

Re: [PATCH v2] i2c-mpc: generate START condition after STOP caused by read i2c_msg

2009-06-14 Thread Ben Dooks
is there a new version of this patch available?

-- 
Ben (b...@fluff.org, http://www.fluff.org/)

  'a smiley only costs 4 bytes'
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev


Re: [PATCH v2] i2c-mpc: generate START condition after STOP caused by read i2c_msg

2009-06-02 Thread Ben Dooks
On Thu, May 28, 2009 at 10:15:22PM +0200, Esben Haabendal wrote:
> On Thu, May 28, 2009 at 9:31 PM, Grant Likely  
> wrote:
> > On Tue, May 26, 2009 at 5:30 AM, Esben Haabendal wrote:
> >> On Tue, May 19, 2009 at 7:22 AM, Esben Haabendal wrote:
> >>> This fixes MAL (arbitration lost) bug caused by illegal use of
> >>> RSTA (repeated START) after STOP condition generated after last byte
> >>> of reads. With this patch, it is possible to do an i2c_transfer() with
> >>> additional i2c_msg's following the I2C_M_RD messages.
> >>>
> >>> It still needs to be resolved if it is possible to fix this issue
> >>> by removing the STOP condition after reads in a robust way.
> >>>
> >>> Signed-off-by: Esben Haabendal 
> >>> ---
> >>> ?drivers/i2c/busses/i2c-mpc.c | ? ?9 +++--
> >>> ?1 files changed, 7 insertions(+), 2 deletions(-)
> >>
> >> Any blockers to get this accepted?
> >
> > It helps if you cc: developers/maintainers of the device. ?ie. Kumar
> > for mpc8xxx, me for 52xx.
> >
> > This is the first time I noticed your posting. ?It will take me a few
> > days before I get a chance to review it.
> 
> Kumar, will you take a look at this patch?

is anyone else likely to review it, or should I merge?

-- 
Ben (b...@fluff.org, http://www.fluff.org/)

  'a smiley only costs 4 bytes'
___
Linuxppc-dev mailing list
Linuxppc-dev@ozlabs.org
https://ozlabs.org/mailman/listinfo/linuxppc-dev


Re: [PATCH v2] i2c-mpc: generate START condition after STOP caused by read i2c_msg

2009-05-26 Thread Ben Dooks
On Tue, May 26, 2009 at 01:30:21PM +0200, Esben Haabendal wrote:
> On Tue, May 19, 2009 at 7:22 AM, Esben Haabendal  
> wrote:
> > This fixes MAL (arbitration lost) bug caused by illegal use of
> > RSTA (repeated START) after STOP condition generated after last byte
> > of reads. With this patch, it is possible to do an i2c_transfer() with
> > additional i2c_msg's following the I2C_M_RD messages.
> >
> > It still needs to be resolved if it is possible to fix this issue
> > by removing the STOP condition after reads in a robust way.
> >
> > Signed-off-by: Esben Haabendal 
> > ---
> > ?drivers/i2c/busses/i2c-mpc.c | ? ?9 +++--
> > ?1 files changed, 7 insertions(+), 2 deletions(-)
> 
> Any blockers to get this accepted?

It would be nice to get an ack from someone who can actually test
the driver before getting this merged.
 
> /Esben
> --
> To unsubscribe from this list: send the line "unsubscribe linux-i2c" in
> the body of a message to majord...@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

-- 
Ben (b...@fluff.org, http://www.fluff.org/)

  'a smiley only costs 4 bytes'
___
Linuxppc-dev mailing list
Linuxppc-dev@ozlabs.org
https://ozlabs.org/mailman/listinfo/linuxppc-dev


Re: [PATCH] i2c-cpm: Pass dev ptr to dma_*_coherent rather than NULL

2009-05-11 Thread Ben Dooks
On Mon, May 11, 2009 at 02:28:50PM +1000, Mark Ware wrote:
> Ben Dooks wrote:
>> On Wed, Apr 29, 2009 at 08:43:14AM -0500, Kumar Gala wrote:
>>> On Apr 22, 2009, at 4:56 PM, Ben Dooks wrote:
>>>
>>>> On Tue, Apr 21, 2009 at 10:11:51AM -0500, Kumar Gala wrote:
>>>>> On Apr 21, 2009, at 7:49 AM, Mark Ware wrote:
>>>>>
>>>>>> Recent DMA changes result in a BUG() when NULL is passed to
>>>>>> dma_alloc_coherent in place of a device.
>>>>>>
>>>>>> Signed-off-by: Mark Ware 
>>>>>> ---
>>>>>>
>>>>>> This patch fixes the BUG() during boot that has appeared during the
>>>>>> 2.6.30 window. It has been tested and appears correct on my 
>>>>>> 8280  based
>>>>>> board.
>>>>>> Sent to both linuxppc-dev and linux-i2c, since I'm not sure where it
>>>>>> belongs.
>>>>>>
>>>>>>
>>>>>> drivers/i2c/busses/i2c-cpm.c |   14 --
>>>>>> 1 files changed, 8 insertions(+), 6 deletions(-)
>>>>> Acked-by: Kumar Gala 
>>>>>
>>>>> Ben, I'm expecting you to pick this up unless you tell me otherwise.
>>>> Yes.
>>> This go in yet?
>>
>> I've had to do a manual apply due to some changes in the
>> driver, so can someone please do a build of my git tree
>> at:
>>
>> git://aeryn.fluff.org.uk/bjdooks/linux.git i2c-for-2630-rc5
>>
>> or tell me which arch and defconfig to build.
>>
>
> Ping.  Is there anything still blocking this?

sorry, have been ill recently, will get this sorted and
pushed.

-- 
Ben (b...@fluff.org, http://www.fluff.org/)

  'a smiley only costs 4 bytes'
___
Linuxppc-dev mailing list
Linuxppc-dev@ozlabs.org
https://ozlabs.org/mailman/listinfo/linuxppc-dev


Re: [PATCH] i2c-cpm: Pass dev ptr to dma_*_coherent rather than NULL

2009-05-03 Thread Ben Dooks
On Wed, Apr 29, 2009 at 08:43:14AM -0500, Kumar Gala wrote:
>
> On Apr 22, 2009, at 4:56 PM, Ben Dooks wrote:
>
>> On Tue, Apr 21, 2009 at 10:11:51AM -0500, Kumar Gala wrote:
>>>
>>> On Apr 21, 2009, at 7:49 AM, Mark Ware wrote:
>>>
>>>> Recent DMA changes result in a BUG() when NULL is passed to
>>>> dma_alloc_coherent in place of a device.
>>>>
>>>> Signed-off-by: Mark Ware 
>>>> ---
>>>>
>>>> This patch fixes the BUG() during boot that has appeared during the
>>>> 2.6.30 window. It has been tested and appears correct on my 8280  
>>>> based
>>>> board.
>>>> Sent to both linuxppc-dev and linux-i2c, since I'm not sure where it
>>>> belongs.
>>>>
>>>>
>>>> drivers/i2c/busses/i2c-cpm.c |   14 --
>>>> 1 files changed, 8 insertions(+), 6 deletions(-)
>>>
>>> Acked-by: Kumar Gala 
>>>
>>> Ben, I'm expecting you to pick this up unless you tell me otherwise.
>>
>> Yes.
>
> This go in yet?

I've had to do a manual apply due to some changes in the
driver, so can someone please do a build of my git tree
at:

git://aeryn.fluff.org.uk/bjdooks/linux.git i2c-for-2630-rc5

or tell me which arch and defconfig to build.

-- 
Ben (b...@fluff.org, http://www.fluff.org/)

  'a smiley only costs 4 bytes'
___
Linuxppc-dev mailing list
Linuxppc-dev@ozlabs.org
https://ozlabs.org/mailman/listinfo/linuxppc-dev


Re: [PATCH] i2c-cpm: Pass dev ptr to dma_*_coherent rather than NULL

2009-04-22 Thread Ben Dooks
On Tue, Apr 21, 2009 at 10:11:51AM -0500, Kumar Gala wrote:
> 
> On Apr 21, 2009, at 7:49 AM, Mark Ware wrote:
> 
> >Recent DMA changes result in a BUG() when NULL is passed to
> >dma_alloc_coherent in place of a device.
> >
> >Signed-off-by: Mark Ware 
> >---
> >
> >This patch fixes the BUG() during boot that has appeared during the
> >2.6.30 window. It has been tested and appears correct on my 8280 based
> >board.
> >Sent to both linuxppc-dev and linux-i2c, since I'm not sure where it  
> >belongs.
> >
> >
> >drivers/i2c/busses/i2c-cpm.c |   14 --
> >1 files changed, 8 insertions(+), 6 deletions(-)
> 
> Acked-by: Kumar Gala 
> 
> Ben, I'm expecting you to pick this up unless you tell me otherwise.

Yes.

-- 
Ben

Q:  What's a light-year?
A:  One-third less calories than a regular year.

___
Linuxppc-dev mailing list
Linuxppc-dev@ozlabs.org
https://ozlabs.org/mailman/listinfo/linuxppc-dev


Re: [PATCH 01/13] sdhci: Add quirk for controllers with no end-of-busy IRQ

2009-02-22 Thread Ben Dooks
On Sat, Feb 21, 2009 at 05:05:04PM +0100, Pierre Ossman wrote:
> On Fri, 20 Feb 2009 20:33:08 +0300
> Anton Vorontsov  wrote:
> 
> > From: Ben Dooks 
> > 
> > The Samsung SDHCI (and FSL eSDHC) controller block seems to fail
> > to generate an INT_DATA_END after the transfer has completed and
> > the bus busy state finished.
> > 
> > Changes in e809517f6fa5803a5a1cd56026f0e2190fc13d5c to use the
> > new busy method are the cause of the behaviour change.
> > 
> > Signed-off-by: Ben Dooks 
> > Signed-off-by: Anton Vorontsov 
> > ---
> 
> Any objections to me merging this right away? It is needed for another
> controller.

none from me.

sorry about the delay, moving servers.

-- 
Ben

Q:  What's a light-year?
A:  One-third less calories than a regular year.

___
Linuxppc-dev mailing list
Linuxppc-dev@ozlabs.org
https://ozlabs.org/mailman/listinfo/linuxppc-dev


Re: [PATCH RFC 0/13] FSL eSDHC support

2009-02-17 Thread Ben Dooks
On Fri, Feb 13, 2009 at 05:46:30PM +0300, Anton Vorontsov wrote:
> Hi all,
> 
> Thanks for the comments on the previous version, here comes another
> RFC...
> 
> Changes since the second RFC:
> - Addressed all comments that were raised by Pierre Ossman.
>   There were too many to mention them all, so here is the link:
>   http://lkml.org/lkml/2009/2/6/320
> 
> Changes since the first RFC:
> - Use of_iomap() in sdhci-of.c (suggested by Arnd Bergmann). Also added
>   Arnd's Acked-by: line for the sdhci-of patch.
> - Kconfig help text improved (thanks to Matt Sealey and M. Warner Losh).
> - In "sdhci: Add quirk to suppress PIO interrupts during DMA transfers"
>   patch: sdhci_init() now clears SDHCI_PIO_DISABLED flag, otherwise we
>   won't disable PIO interrupts after suspend.
> - New patch: "sdhci: Add type checking for IO memory accessors"

It would be useful to know what Pierre thinks of these, I would like
to update the sdhci-s3c binding before the next merge window.

-- 
Ben (b...@fluff.org, http://www.fluff.org/)

  'a smiley only costs 4 bytes'
___
Linuxppc-dev mailing list
Linuxppc-dev@ozlabs.org
https://ozlabs.org/mailman/listinfo/linuxppc-dev


Re: [PATCH] i2c-mpc: do not allow interruptions when waiting for I2C to complete

2009-02-10 Thread Ben Dooks
On Fri, Feb 06, 2009 at 08:00:37AM -0600, Timur Tabi wrote:
> The i2c_wait() function is using wait_event_interruptible_timeout() to wait 
> for
> the I2C controller to signal that it has completed an I2C bus operation.  If
> the process that causes the I2C operation terminated abruptly, the wait will
> be interrupted, returning an error.  It is better to let the I2C operation
> finished before the process exits.
> 
> It is safe to use wait_event_timeout() instead, because the timeout will allow
> the process to exit if the I2C bus hangs.  It's also better to allow the
> I2C operation to finish, because unacknowledged I2C operations can cause the
> I2C bus to hang.
> 
> Signed-off-by: Timur Tabi 
Acked-by: Ben Dooks 
> ---
> 
> A similar change should probably be done to i2c-cpm.c, and maybe all other
> I2C drivers.  Not many use wait_event_interruptible_timeout().
> 
>  drivers/i2c/busses/i2c-mpc.c |9 +++--
>  1 files changed, 3 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/i2c/busses/i2c-mpc.c b/drivers/i2c/busses/i2c-mpc.c
> index a9a45fc..c0ace48 100644
> --- a/drivers/i2c/busses/i2c-mpc.c
> +++ b/drivers/i2c/busses/i2c-mpc.c
> @@ -70,7 +70,7 @@ static irqreturn_t mpc_i2c_isr(int irq, void *dev_id)
>   /* Read again to allow register to stabilise */
>   i2c->interrupt = readb(i2c->base + MPC_I2C_SR);
>   writeb(0, i2c->base + MPC_I2C_SR);
> - wake_up_interruptible(&i2c->queue);
> + wake_up(&i2c->queue);
>   }
>   return IRQ_HANDLED;
>  }
> @@ -115,13 +115,10 @@ static int i2c_wait(struct mpc_i2c *i2c, unsigned 
> timeout, int writing)
>   writeb(0, i2c->base + MPC_I2C_SR);
>   } else {
>   /* Interrupt mode */
> - result = wait_event_interruptible_timeout(i2c->queue,
> + result = wait_event_timeout(i2c->queue,
>   (i2c->interrupt & CSR_MIF), timeout * HZ);
>  
> - if (unlikely(result < 0)) {
> - pr_debug("I2C: wait interrupted\n");
> - writeccr(i2c, 0);
> - } else if (unlikely(!(i2c->interrupt & CSR_MIF))) {
> + if (unlikely(!(i2c->interrupt & CSR_MIF))) {
>   pr_debug("I2C: wait timeout\n");
>   writeccr(i2c, 0);
>   result = -ETIMEDOUT;
> -- 
> 1.5.5
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-i2c" in
> the body of a message to majord...@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

-- 
Ben (b...@fluff.org, http://www.fluff.org/)

  'a smiley only costs 4 bytes'
___
Linuxppc-dev mailing list
Linuxppc-dev@ozlabs.org
https://ozlabs.org/mailman/listinfo/linuxppc-dev


Re: [PATCH] i2c-mpc: do not allow interruptions when waiting for I2C to complete

2009-02-10 Thread Ben Dooks
On Tue, Feb 10, 2009 at 10:21:04AM -0600, Timur Tabi wrote:
> Jean Delvare wrote:
> 
> > To the best of my knowledge i2c-mpc falls into the "embedded platforms"
> > category and is thus under Ben's responsibility.
> 
> I don't see his Signed-off-by on any patch to i2c-mpc.c for the past
> three years (at least).  I do see yours, however.  You have directly
> applied similar patches in the recent past, so of course I will assume
> that you would apply this one.

My appointment as an embedded maintainer is relatively recent.

I do try and review patches, and sometimes these patches get sent
to another maintainer.

-- 
Ben (b...@fluff.org, http://www.fluff.org/)

  'a smiley only costs 4 bytes'
___
Linuxppc-dev mailing list
Linuxppc-dev@ozlabs.org
https://ozlabs.org/mailman/listinfo/linuxppc-dev


Re: [PATCH] mmc: Add driver for Freescale eSDHC controllers

2009-01-15 Thread Ben Dooks
On Thu, Jan 15, 2009 at 02:56:05AM +0300, Anton Vorontsov wrote:
> On Thu, Jan 15, 2009 at 07:37:00AM +0800, Liu Dave wrote:
> > > This patch adds support for the Freescale Enhanced Secure Digital
> > > Host Controller Interface as found in some Freescale PowerPC
> > > microprocessors (e.g. MPC837x SOCs).
> > 
> > The Freescale ESDHC controller is basically compatible with the
> > standard SDHC controller. but the eshdc expand some bits.
> > 
> > The esdhc.c is much like the orignal sdhci.c.
> > it is possible to merge them.
> 
> Ah. I wonder why Freescale just didn't write some patches for sdhci,
> but copied the code instead...

For the same reasons earthmen like tea.

-- 
Ben (b...@fluff.org, http://www.fluff.org/)

  'a smiley only costs 4 bytes'
___
Linuxppc-dev mailing list
Linuxppc-dev@ozlabs.org
https://ozlabs.org/mailman/listinfo/linuxppc-dev


Re: Please pull bjdoo ks' i2c-fixes

2008-12-18 Thread Ben Dooks
On Wed, Dec 17, 2008 at 12:55:06PM +0100, Jochen Friedrich wrote:
> Hi Ben,
> 
> > Mike Ditto (1):
> >   i2c-cpm: Detect and report NAK right away instead of timing out
> 
> Could you also have a look at http://patchwork.ozlabs.org/patch/7452/ ?
> (Discussion at http://patchwork.ozlabs.org/patch// )
> 
> IIRC, Kumar prefers to push this via your tree.

Ok, but this isn't really a fix, so I'll look at putting it into my
i2c-next tree, not i2c-fixes.

-- 
Ben (b...@fluff.org, http://www.fluff.org/)

  'a smiley only costs 4 bytes'
___
Linuxppc-dev mailing list
Linuxppc-dev@ozlabs.org
https://ozlabs.org/mailman/listinfo/linuxppc-dev


Re: [PATCH] i2c-cpm: Detect and report NAK right away instead of timing out.

2008-11-02 Thread Ben Dooks
On Fri, Oct 31, 2008 at 05:29:25PM -0700, Mike Ditto wrote:
> Make the driver report an ENXIO error immediately upon NAK instead of
> waiting for another interrupt and getting a timeout.
> 
> Signed-off-by: Mike Ditto <[EMAIL PROTECTED]>
> ---
> When reading from a device that is not present or declines to respond
> to, e.g., a non-existent register address, CPM immediately reports a
> NAK condition in the TxBD, but the driver kept waiting until a timeout,
> which takes 1 second and causes an ugly console error message.

hmm, that block of text could probably go into the patch description.

It looks ok, I'll merge and push out to linus with the s3c fixes.
 
> Index: linux/drivers/i2c/busses/i2c-cpm.c
> ===
> retrieving revision 1.3
> diff -u -p -r1.3 i2c-cpm.c
> --- linux/drivers/i2c/busses/i2c-cpm.c31 Oct 2008 06:36:08 -  
> 1.3
> +++ linux/drivers/i2c/busses/i2c-cpm.c1 Nov 2008 00:12:45 -
> @@ -369,6 +369,7 @@ static int cpm_i2c_xfer(struct i2c_adapt
>   pmsg = &msgs[tptr];
>   if (pmsg->flags & I2C_M_RD)
>   ret = wait_event_interruptible_timeout(cpm->i2c_wait,
> + (in_be16(&tbdf[tptr].cbd_sc) & BD_SC_NAK) ||
>   !(in_be16(&rbdf[rptr].cbd_sc) & BD_SC_EMPTY),
>   1 * HZ);
>   else
> --
> To unsubscribe from this list: send the line "unsubscribe linux-i2c" in
> the body of a message to [EMAIL PROTECTED]
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

-- 
Ben ([EMAIL PROTECTED], http://www.fluff.org/)

  'a smiley only costs 4 bytes'
--
To unsubscribe from this list: send the line "unsubscribe linux-i2c" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [i2c] [PATCH 2/2] i2c: MPC8349E-mITX Power Management and GPIO expander driver

2008-09-22 Thread Ben Dooks
On Fri, Sep 19, 2008 at 10:03:54PM +0400, Anton Vorontsov wrote:
> On MPC8349E-mITX, MPC8315E-RDB and MPC837x-RDB boards there is a
> Freescale MC9S08QG8 (MCU) chip with the custom firmware
> pre-programmed. The chip is used to power-off the board by the
> software, and to control some GPIO pins.

I suppose there's no other place for this sort of thing to go at
the moment other than drivers/i2c/chips.

I've also a driver for a similar PMU fitted to a number
of our boards that provides basic gpio (wakeup sources) and some
minimalist power management (so isn't really fit for regulator
framwork) and isn't really a gpio chip (so drivers/gpio isn't
really the place for it either.)
 
> Signed-off-by: Anton Vorontsov <[EMAIL PROTECTED]>
> ---
>  drivers/i2c/chips/Kconfig|   11 ++
>  drivers/i2c/chips/Makefile   |1 +
>  drivers/i2c/chips/mcu_mpc8349emitx.c |  213 
> ++
>  3 files changed, 225 insertions(+), 0 deletions(-)
>  create mode 100644 drivers/i2c/chips/mcu_mpc8349emitx.c
> 
> diff --git a/drivers/i2c/chips/Kconfig b/drivers/i2c/chips/Kconfig
> index a95cb94..1735682 100644
> --- a/drivers/i2c/chips/Kconfig
> +++ b/drivers/i2c/chips/Kconfig
> @@ -172,4 +172,15 @@ config MENELAUS
> and other features that are often used in portable devices like
> cell phones and PDAs.
>  
> +config MCU_MPC8349EMITX
> + tristate "MPC8349E-mITX MCU driver"
> + depends on I2C && PPC_83xx
> + select GENERIC_GPIO
> + select ARCH_REQUIRE_GPIOLIB
> + help
> +   Say Y here to enable soft power-off functionality on the Freescale
> +   boards with the MPC8349E-mITX-compatible MCU chips. This driver will
> +   also register MCU GPIOs with the generic GPIO API, so you'll able
> +   to use MCU pins as GPIOs.
> +
>  endmenu
> diff --git a/drivers/i2c/chips/Makefile b/drivers/i2c/chips/Makefile
> index 39e3e69..ca520fa 100644
> --- a/drivers/i2c/chips/Makefile
> +++ b/drivers/i2c/chips/Makefile
> @@ -21,6 +21,7 @@ obj-$(CONFIG_ISP1301_OMAP)  += isp1301_omap.o
>  obj-$(CONFIG_TPS65010)   += tps65010.o
>  obj-$(CONFIG_MENELAUS)   += menelaus.o
>  obj-$(CONFIG_SENSORS_TSL2550)+= tsl2550.o
> +obj-$(CONFIG_MCU_MPC8349EMITX)   += mcu_mpc8349emitx.o
>  
>  ifeq ($(CONFIG_I2C_DEBUG_CHIP),y)
>  EXTRA_CFLAGS += -DDEBUG
> diff --git a/drivers/i2c/chips/mcu_mpc8349emitx.c 
> b/drivers/i2c/chips/mcu_mpc8349emitx.c
> new file mode 100644
> index 000..47b07b8
> --- /dev/null
> +++ b/drivers/i2c/chips/mcu_mpc8349emitx.c
> @@ -0,0 +1,213 @@
> +/*
> + * Power Management and GPIO expander driver for MPC8349E-mITX-compatible MCU
> + *
> + * Copyright (c) 2008  MontaVista Software, Inc.
> + *
> + * Author: Anton Vorontsov <[EMAIL PROTECTED]>
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License as published by
> + * the Free Software Foundation; either version 2 of the License, or
> + * (at your option) any later version.
> + */
> +
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +
> +/*
> + * I don't have specifications for the MCU firmware, I found this register
> + * and bits positions by the trial&error method.
> + */
> +#define MCU_REG_CTRL 0x20
> +#define MCU_CTRL_POFF0x40
> +
> +#define MCU_NUM_GPIO 2
> +
> +struct mcu {
> + struct mutex lock;
> + struct device_node *np;
> + struct i2c_client *client;
> + struct of_gpio_chip of_gc;
> + u8 reg_ctrl;
> +};
> +
> +static struct mcu *glob_mcu;
> +
> +static void mcu_power_off(void)
> +{
> + struct mcu *mcu = glob_mcu;
> +
> + pr_info("Sending power-off request to the MCU...\n");
> + mutex_lock(&mcu->lock);
> + i2c_smbus_write_byte_data(glob_mcu->client, MCU_REG_CTRL,
> +   mcu->reg_ctrl | MCU_CTRL_POFF);
> + mutex_unlock(&mcu->lock);
> +}
> +
> +static void mcu_gpio_set(struct gpio_chip *gc, unsigned int gpio, int val)
> +{
> + struct of_gpio_chip *of_gc = to_of_gpio_chip(gc);
> + struct mcu *mcu = container_of(of_gc, struct mcu, of_gc);
> + u8 bit = 1 << (4 + gpio);
> +
> + mutex_lock(&mcu->lock);
> + if (val)
> + mcu->reg_ctrl &= ~bit;
> + else
> + mcu->reg_ctrl |= bit;
> +
> + i2c_smbus_write_byte_data(mcu->client, MCU_REG_CTRL, mcu->reg_ctrl);
> + mutex_unlock(&mcu->lock);
> +}
> +
> +static int mcu_gpio_dir_out(struct gpio_chip *gc, unsigned int gpio, int val)
> +{
> + mcu_gpio_set(gc, gpio, val);
> + return 0;
> +}
> +
> +static int mcu_gpiochip_add(struct mcu *mcu)
> +{
> + struct device_node *np;
> + struct of_gpio_chip *of_gc = &mcu->of_gc;
> + struct gpio_chip *gc = &of_gc->gc;
> + int ret;
> +
> + np = of_find_compatible_node(NULL, NULL, "fsl,mcu-mpc8349emitx");
> + if (!np)
> + return -ENODEV;
> +

Re: [PATCH] pata_of_platform: fix no irq handling

2008-08-11 Thread Ben Dooks

On Mon, Aug 11, 2008 at 07:19:13PM +0400, Anton Vorontsov wrote:
> When no irq specified, pata_of_platform fills irq_res with -1,
> which is wrong to do for two reasons:
> 
> 1. By definition, 'no irq' should be IRQ 0, not some negative integer;

interesting, IRQ 0 is actually valid on some ARM systems.

> 2. pata_platform checks for irq_res.start > 0, but since irq_res.start
>is unsigned type, the check will be true for `-1'.
> 
> Reported-by: Steven A. Falco <[EMAIL PROTECTED]>
> Signed-off-by: Anton Vorontsov <[EMAIL PROTECTED]>
> ---
> 
> On Mon, Aug 11, 2008 at 10:48:50AM -0400, Steven A. Falco wrote:
> > I think there is a bug in the communications between pata_of_platform
> > and pata_platform.  I will refer to the master branch of the DENX git
> > tree, which is roughly v2.6.26.1 at this time.  I am using a Sequoia
> > board with a PPC440EPx.
> > 
> > In pata_of_platform, we have:
> > 
> > ret = of_irq_to_resource(dn, 0, &irq_res);
> > if (ret == NO_IRQ)
> > irq_res.start = irq_res.end = -1;
> > 
> > so if there is no interrupt defined, then start and end are -1. 
> > However, __pata_platform_probe has:
> > 
> > if (irq_res && irq_res->start > 0) {
> > irq = irq_res->start;
> > irq_flags = irq_res->flags;
> > }
> > 
> > You might think that the (irq_res->start > 0) test will fail, as it
> > should in this no-irq case.  But, start is a u64, so the -1 actually
> > looks like a large positive number in the comparison.  So,
> > __pata_platform_probe attempts to use an interrupt when there isn't one.
> > 
> > I think the fix would be to change __pata_platform_probe to:
> > 
> > if (irq_res && irq_res->start != -1) {
> > 
> > but that might have other unintended consequences, so I'll defer to
> > whomever knows more about the intent of this code.
> 
> Something like this patch should work. Thanks for noticing!
> 
>  drivers/ata/pata_of_platform.c |2 +-
>  1 files changed, 1 insertions(+), 1 deletions(-)
> 
> diff --git a/drivers/ata/pata_of_platform.c b/drivers/ata/pata_of_platform.c
> index 408da30..1f18ad9 100644
> --- a/drivers/ata/pata_of_platform.c
> +++ b/drivers/ata/pata_of_platform.c
> @@ -52,7 +52,7 @@ static int __devinit pata_of_platform_probe(struct 
> of_device *ofdev,
>  
>   ret = of_irq_to_resource(dn, 0, &irq_res);
>   if (ret == NO_IRQ)
> - irq_res.start = irq_res.end = -1;
> + irq_res.start = irq_res.end = 0;
>   else
>   irq_res.flags = 0;
>  

-- 
Ben

Q:  What's a light-year?
A:  One-third less calories than a regular year.

___
Linuxppc-dev mailing list
Linuxppc-dev@ozlabs.org
https://ozlabs.org/mailman/listinfo/linuxppc-dev


Re: libata badness

2008-08-01 Thread Ben Dooks
On Thu, Jul 31, 2008 at 04:58:05PM -0500, Kumar Gala wrote:
> I figured out the issue.  Its related to the interrupt routing being  
> conveyed to the code from the device tree.  We have a missing 'space'  
> causing issues.

out of interest, is the M5229 attached to a single IRQ in this device
or has the bridge been configured to route the IRQs from the IDE to
the same pin on the PIC?

-- 
Ben

Q:  What's a light-year?
A:  One-third less calories than a regular year.
___
Linuxppc-dev mailing list
Linuxppc-dev@ozlabs.org
https://ozlabs.org/mailman/listinfo/linuxppc-dev


Re: libata badness

2008-07-31 Thread Ben Dooks
On Thu, Jul 31, 2008 at 01:53:45PM -0500, Kumar Gala wrote:
> I'm getting the following badness with top of tree on a embedded  
> PowerPC w/a ULI 1575 bridge (M5229 IDE):
> 
> 02:1f.0 IDE interface: Acer Laboratories Inc. [ALi] M5229 IDE (rev c8)
> 
> If you need more info let me know.
> 
> - k
> 
> ahci :02:1f.1: AHCI 0001. 32 slots 4 ports 3 Gbps 0xf impl  
> SATA mode
> ahci :02:1f.1: flags: ncq sntf ilck pm led clo pmp pio slum part
> [ cut here ]
> Badness at c021e700 [verbose debug info unavailable]

it would be helpful to compile a kernel with verbose fault information
turned on.

> NIP: c021e700 LR: c021e6e8 CTR: c022ce90
> REGS: ef82bca0 TRAP: 0700   Not tainted  (2.6.27-rc1-00495-g33bd9fe- 
> dirty)
> MSR: 00029032   CR: 22044022  XER: 2000
> TASK = ef83[1] 'swapper' THREAD: ef82a000 CPU: 1
> GPR00: 0001 ef82bd50 ef83  9032  ef0b64fc  
> 
> GPR08: ef937c10 c022f93f efbfc8e0 ef900b60 22044028 fffdd3ff 0ffe8700  
> c044c17c
> GPR16: c04d52ec ef82f458 ef82f4f4 ef82f4f4 c044c234 c044c5d8 c044c5d8  
> c044c220
> GPR24: c044c218 c04d52f0 0080 c022f940  c044c244 efbec490  
> 
> NIP [c021e700] ata_host_activate+0x40/0x10c
> LR [c021e6e8] ata_host_activate+0x28/0x10c
> Call Trace:
> [ef82bd50] [c021e6e8] ata_host_activate+0x28/0x10c (unreliable)
> [ef82bd80] [c022f48c] ahci_init_one+0x8b4/0xd68
> [ef82be30] [c01b69c0] pci_device_probe+0x84/0xa8
> [ef82be50] [c01e5438] driver_probe_device+0xb4/0x1e8
> [ef82be70] [c01e55f0] __driver_attach+0x84/0x88
> [ef82be90] [c01e4ba0] bus_for_each_dev+0x5c/0xa4
> [ef82bec0] [c01e5254] driver_attach+0x24/0x34
> [ef82bed0] [c01e44b8] bus_add_driver+0x1d8/0x24c
> [ef82bef0] [c01e5810] driver_register+0x70/0x160
> [ef82bf10] [c01b6c54] __pci_register_driver+0x64/0xc4
> [ef82bf30] [c04aaa60] ahci_init+0x28/0x38
> [ef82bf40] [c048717c] do_one_initcall+0x38/0x1ac
> [ef82bfb0] [c04874e0] kernel_init+0x1f0/0x1fc
> [ef82bff0] [c0013b04] kernel_thread+0x44/0x60
> Instruction dump:
> 7cbb2b78 90010034 7cda3378 7cf93b78 7c7e1b78 4bff9dbd 7c7f1b79 408200c8
> 2f9c 409e002c 313b 7c09d910 <0f00> 80010034 7fc3f378  
> 7f24cb78
> scsi0 : ahci
> scsi1 : ahci
> scsi2 : ahci
> scsi3 : ahci
> ata1: SATA max UDMA/133 abar [EMAIL PROTECTED] port 0x80006100
> ata2: SATA max UDMA/133 abar [EMAIL PROTECTED] port 0x80006180
> ata3: SATA max UDMA/133 abar [EMAIL PROTECTED] port 0x80006200
> ata4: SATA max UDMA/133 abar [EMAIL PROTECTED] port 0x80006280
> ata1: SATA link down (SStatus 0 SControl 300)
> ata2: SATA link up 1.5 Gbps (SStatus 113 SControl 300)
> ata2.00: qc timeout (cmd 0xec)
> ata2.00: failed to IDENTIFY (I/O error, err_mask=0x4)
> ata2: SATA link up 1.5 Gbps (SStatus 113 SControl 300)
> ata2.00: qc timeout (cmd 0xec)
> ata2.00: failed to IDENTIFY (I/O error, err_mask=0x4)
> ata2: SATA link up 1.5 Gbps (SStatus 113 SControl 300)
> ata2.00: qc timeout (cmd 0xec)
> ata2.00: failed to IDENTIFY (I/O error, err_mask=0x4)
> ata2: SATA link up 1.5 Gbps (SStatus 113 SControl 300)
> ata3: SATA link down (SStatus 0 SControl 300)
> ata4: SATA link down (SStatus 0 SControl 300)
> scsi4 : pata_ali
> scsi5 : pata_ali
> ata5: PATA max UDMA/133 cmd 0x1200 ctl 0x1208 bmdma 0x1220
> ata6: PATA max UDMA/133 cmd 0x1210 ctl 0x1218 bmdma 0x1228

Looks like you're running into the same problem as I did, with the
fact that the M5529 is in native mode, but doesn't have any IRQ
available. Do you know if the bridge chip in it is in routes the
IRQs internally?

> ata5.00: ATAPI: TSSTcorp CDW/DVD SH-M522C, TS06, max UDMA/33
> ata5.00: WARNING: ATAPI DMA disabled for reliablity issues.  It can be  
> enabled
> ata5.00: WARNING: via pata_ali.atapi_dma modparam or corresponding  
> sysfs node.
> ata5.00: configured for UDMA/33
> ata5.00: exception Emask 0x0 SAct 0x0 SErr 0x0 action 0x6 frozen
> ata5.00: cmd a0/00:00:00:24:00/00:00:00:00:00/a0 tag 0 pio 36 in
>  cdb 12 00 00 00 24 00 00 00  00 00 00 00 00 00 00 00
>  res 40/00:00:00:00:00/00:00:00:00:00/00 Emask 0x4 (timeout)
> ata5.00: status: { DRDY }
> ata5: soft resetting link
> ata5.00: configured for UDMA/33
> ata5: EH complete
> ata5.00: limiting speed to UDMA/25:PIO4
> ata5.00: exception Emask 0x0 SAct 0x0 SErr 0x0 action 0x6 frozen
> ata5.00: cmd a0/00:00:00:24:00/00:00:00:00:00/a0 tag 0 pio 36 in
>  cdb 12 00 00 00 24 00 00 00  00 00 00 00 00 00 00 00
>  res 40/00:00:00:00:00/00:00:00:00:00/00 Emask 0x4 (timeout)
> ata5.00: status: { DRDY }
> ata5: soft resetting link
> ata5.00: configured for UDMA/25
> ata5: EH complete
> ata5.00: limiting speed to PIO4
> ata5.00: exception Emask 0x0 SAct 0x0 SErr 0x0 action 0x6 frozen
> ata5.00: cmd a0/00:00:00:24:00/00:00:00:00:00/a0 tag 0 pio 36 in
>  cdb 12 00 00 00 24 00 00 00  00 00 00 00 00 00 00 00
>  res 40/00:00:00:00:00/00:00:00:00:00/00 Emask 0x4 (timeout)
> ata5.00: status: { DRDY }
> ata5: soft resetting link
> ata5.00: configured for PIO4
> ata5: EH c

Re: [PATCH 3/3 2.6.24-git] PPC/LIBATA: Select HAVE_PATA_PLATFORM for ARCH_PPC

2008-02-02 Thread Ben Dooks
On Sat, Feb 02, 2008 at 01:12:39PM -0600, Olof Johansson wrote:
> On Sat, Feb 02, 2008 at 04:21:36PM +0000, Ben Dooks wrote:
> > Use the new HAVE_PATA_PLATFORM to select PATA_PLATFORM
> > driver.
> > 
> > CC: linuxppc-dev@ozlabs.org
> > Signed-off-by: Ben Dooks <[EMAIL PROTECTED]>
> 
> What tree is this against? It doesn't apply to current mainline nor
> jgarzik's libata-dev upstream branch.

It is a series that i've just sent to the linux-ide list, only the ppc
part was cc'd to the ppc list.
 
> Anyway: NACK: I like this approach but this needs to be added to
> arch/powerpc as well.

I can add that to the series.
 
> It's actually more needed there than arch/ppc, I don't personally care
> if arch/ppc can't use pata_platform for the last couple of months it's
> still around (it's getting deleted this summer, all platforms should
> have moved to powerpc by then).

Thanks, didn't know that.

-- 
Ben

Q:  What's a light-year?
A:  One-third less calories than a regular year.

___
Linuxppc-dev mailing list
Linuxppc-dev@ozlabs.org
https://ozlabs.org/mailman/listinfo/linuxppc-dev


[PATCH 3/3 2.6.24-git] PPC/LIBATA: Select HAVE_PATA_PLATFORM for ARCH_PPC

2008-02-02 Thread Ben Dooks
Use the new HAVE_PATA_PLATFORM to select PATA_PLATFORM
driver.

CC: linuxppc-dev@ozlabs.org
Signed-off-by: Ben Dooks <[EMAIL PROTECTED]>

Index: linux-2.6.24-git12-pata2/arch/ppc/Kconfig
===
--- linux-2.6.24-git12-pata2.orig/arch/ppc/Kconfig
+++ linux-2.6.24-git12-pata2/arch/ppc/Kconfig
@@ -41,6 +41,7 @@ config GENERIC_CALIBRATE_DELAY
 
 config PPC
bool
+   select HAVE_PATA_PLATFORM
default y
 
 config PPC32
Index: linux-2.6.24-git12-pata2/drivers/ata/Kconfig
===
--- linux-2.6.24-git12-pata2.orig/drivers/ata/Kconfig
+++ linux-2.6.24-git12-pata2/drivers/ata/Kconfig
@@ -624,7 +624,7 @@ config HAVE_PATA_PLATFORM
 
 config PATA_PLATFORM
tristate "Generic platform device PATA support"
-   depends on EMBEDDED || PPC || HAVE_PATA_PLATFORM
+   depends on EMBEDDED || HAVE_PATA_PLATFORM
help
  This option enables support for generic directly connected ATA
  devices commonly found on embedded systems.

-- 
Ben ([EMAIL PROTECTED], http://www.fluff.org/)

  'a smiley only costs 4 bytes'
___
Linuxppc-dev mailing list
Linuxppc-dev@ozlabs.org
https://ozlabs.org/mailman/listinfo/linuxppc-dev