Re: [PATCH 0/6] ipmi: Convert to platform remove callback returning void

[Corey Minyard]

On Thu, Apr 11, 2024 at 09:15:03AM +0200, Uwe Kleine-König wrote:
> Hello,
> 
> On Tue, Mar 05, 2024 at 05:26:57PM +0100, Uwe Kleine-König wrote:
> > this series converts all drivers below drivers/char/ipmi to struct
> > platform_driver::remove_new(). See commit 5c5a7680e67b ("platform: Provide a
> > remove callback that returns no value") for an extended explanation and the
> > eventual goal.
> > 
> > All conversations are trivial, because their .remove() callbacks
> > returned zero unconditionally.
> > 
> > There are no interdependencies between these patches, so they could be
> > picked up individually. But I'd hope that they get picked up all
> > together by Corey.

Yeah, I was kind of waiting for more reviews, but this is pretty
straightforward.  I've pulled this into my tree.

-corey

> 
> Apart from a (positive) review reply I didn't get any feedback to this
> series. My quest to change the prototype of struct
> platform_driver::remove depends on these patches, so it would be great
> if they made it in during the next merge window.
> 
> Best regards
> Uwe
> 
> -- 
> Pengutronix e.K.   | Uwe Kleine-König|
> Industrial Linux Solutions | https://www.pengutronix.de/ |

Re: [PATCH 6/9] ipmi: Convert from tasklet to BH workqueue

[Corey Minyard]

On Thu, Mar 28, 2024 at 12:41:22PM -0700, Allen wrote:
> > > > I believe that work queues items are execute single-threaded for a work
> > > > queue, so this should be good.  I need to test this, though.  It may be
> > > > that an IPMI device can have its own work queue; it may not be important
> > > > to run it in bh context.
> > >
> > >   Fair point. Could you please let me know once you have had a chance to 
> > > test
> > > these changes. Meanwhile, I will work on RFC wherein IPMI will have its 
> > > own
> > > workqueue.
> > >
> > >  Thanks for taking time out to review.
> >
> > After looking and thinking about it a bit, a BH context is still
> > probably the best for this.
> >
> > I have tested this patch under load and various scenarios and it seems
> > to work ok.  So:
> >
> > Tested-by: Corey Minyard 
> > Acked-by: Corey Minyard 
> >
> > Or I can take this into my tree.
> >
> > -corey
> 
>  Thank you very much. I think it should be okay for you to carry it into
> your tree.

Ok, it's in my for-next tree.  I fixed the directory reference, and I
changed all the comments where you changed "tasklet" to "work" to
instead say "workqueue".

-corey

> 
> - Allen
> 

Re: [PATCH 6/9] ipmi: Convert from tasklet to BH workqueue

[Corey Minyard]

On Thu, Mar 28, 2024 at 10:52:16AM -0700, Allen wrote:
> On Wed, Mar 27, 2024 at 11:05 AM Corey Minyard  wrote:
> >
> > I believe that work queues items are execute single-threaded for a work
> > queue, so this should be good.  I need to test this, though.  It may be
> > that an IPMI device can have its own work queue; it may not be important
> > to run it in bh context.
> 
>   Fair point. Could you please let me know once you have had a chance to test
> these changes. Meanwhile, I will work on RFC wherein IPMI will have its own
> workqueue.
> 
>  Thanks for taking time out to review.

After looking and thinking about it a bit, a BH context is still
probably the best for this.

I have tested this patch under load and various scenarios and it seems
to work ok.  So:

Tested-by: Corey Minyard 
Acked-by: Corey Minyard 

Or I can take this into my tree.

-corey

> 
> - Allen
> 
> >
> > -corey
> >
> > >
> > > Based on the work done by Tejun Heo 
> > > Branch: https://git.kernel.org/pub/scm/linux/kernel/git/tj/wq.git for-610
> > >
> > > Signed-off-by: Allen Pais 
> > > ---
> > >  drivers/char/ipmi/ipmi_msghandler.c | 30 ++---
> > >  1 file changed, 15 insertions(+), 15 deletions(-)
> > >
> > > diff --git a/drivers/char/ipmi/ipmi_msghandler.c 
> > > b/drivers/char/ipmi/ipmi_msghandler.c
> > > index b0eedc4595b3..fce2a2dbdc82 100644
> > > --- a/drivers/char/ipmi/ipmi_msghandler.c
> > > +++ b/drivers/char/ipmi/ipmi_msghandler.c
> > > @@ -36,12 +36,13 @@
> > >  #include 
> > >  #include 
> > >  #include 
> > > +#include 
> > >
> > >  #define IPMI_DRIVER_VERSION "39.2"
> > >
> > >  static struct ipmi_recv_msg *ipmi_alloc_recv_msg(void);
> > >  static int ipmi_init_msghandler(void);
> > > -static void smi_recv_tasklet(struct tasklet_struct *t);
> > > +static void smi_recv_work(struct work_struct *t);
> > >  static void handle_new_recv_msgs(struct ipmi_smi *intf);
> > >  static void need_waiter(struct ipmi_smi *intf);
> > >  static int handle_one_recv_msg(struct ipmi_smi *intf,
> > > @@ -498,13 +499,13 @@ struct ipmi_smi {
> > >   /*
> > >* Messages queued for delivery.  If delivery fails (out of memory
> > >* for instance), They will stay in here to be processed later in a
> > > -  * periodic timer interrupt.  The tasklet is for handling received
> > > +  * periodic timer interrupt.  The work is for handling received
> > >* messages directly from the handler.
> > >*/
> > >   spinlock_t   waiting_rcv_msgs_lock;
> > >   struct list_head waiting_rcv_msgs;
> > >   atomic_t watchdog_pretimeouts_to_deliver;
> > > - struct tasklet_struct recv_tasklet;
> > > + struct work_struct recv_work;
> > >
> > >   spinlock_t xmit_msgs_lock;
> > >   struct list_head   xmit_msgs;
> > > @@ -704,7 +705,7 @@ static void clean_up_interface_data(struct ipmi_smi 
> > > *intf)
> > >   struct cmd_rcvr  *rcvr, *rcvr2;
> > >   struct list_head list;
> > >
> > > - tasklet_kill(>recv_tasklet);
> > > + cancel_work_sync(>recv_work);
> > >
> > >   free_smi_msg_list(>waiting_rcv_msgs);
> > >   free_recv_msg_list(>waiting_events);
> > > @@ -1319,7 +1320,7 @@ static void free_user(struct kref *ref)
> > >  {
> > >   struct ipmi_user *user = container_of(ref, struct ipmi_user, 
> > > refcount);
> > >
> > > - /* SRCU cleanup must happen in task context. */
> > > + /* SRCU cleanup must happen in work context. */
> > >   queue_work(remove_work_wq, >remove_work);
> > >  }
> > >
> > > @@ -3605,8 +3606,7 @@ int ipmi_add_smi(struct module *owner,
> > >   intf->curr_seq = 0;
> > >   spin_lock_init(>waiting_rcv_msgs_lock);
> > >   INIT_LIST_HEAD(>waiting_rcv_msgs);
> > > - tasklet_setup(>recv_tasklet,
> > > -  smi_recv_tasklet);
> > > + INIT_WORK(>recv_work, smi_recv_work);
> > >   atomic_set(>watchdog_pretimeouts_to_deliver, 0);
> > >   spin_lock_init(>xmit_msgs_lock);
> > >   INIT_LIST_HEAD(>xmit_msgs);
> > > @@ -4779,7 +4779,7 @@ static void handle_new_recv_msgs(struct ipmi_smi 
> > > *intf)
> > >* To preser

Re: [PATCH 6/9] ipmi: Convert from tasklet to BH workqueue

[Corey Minyard]

On Wed, Mar 27, 2024 at 04:03:11PM +, Allen Pais wrote:
> The only generic interface to execute asynchronously in the BH context is
> tasklet; however, it's marked deprecated and has some design flaws. To
> replace tasklets, BH workqueue support was recently added. A BH workqueue
> behaves similarly to regular workqueues except that the queued work items
> are executed in the BH context.
> 
> This patch converts drivers/infiniband/* from tasklet to BH workqueue.

I think you mean drivers/char/ipmi/* here.

I believe that work queues items are execute single-threaded for a work
queue, so this should be good.  I need to test this, though.  It may be
that an IPMI device can have its own work queue; it may not be important
to run it in bh context.

-corey

> 
> Based on the work done by Tejun Heo 
> Branch: https://git.kernel.org/pub/scm/linux/kernel/git/tj/wq.git for-6.10
> 
> Signed-off-by: Allen Pais 
> ---
>  drivers/char/ipmi/ipmi_msghandler.c | 30 ++---
>  1 file changed, 15 insertions(+), 15 deletions(-)
> 
> diff --git a/drivers/char/ipmi/ipmi_msghandler.c 
> b/drivers/char/ipmi/ipmi_msghandler.c
> index b0eedc4595b3..fce2a2dbdc82 100644
> --- a/drivers/char/ipmi/ipmi_msghandler.c
> +++ b/drivers/char/ipmi/ipmi_msghandler.c
> @@ -36,12 +36,13 @@
>  #include 
>  #include 
>  #include 
> +#include 
>  
>  #define IPMI_DRIVER_VERSION "39.2"
>  
>  static struct ipmi_recv_msg *ipmi_alloc_recv_msg(void);
>  static int ipmi_init_msghandler(void);
> -static void smi_recv_tasklet(struct tasklet_struct *t);
> +static void smi_recv_work(struct work_struct *t);
>  static void handle_new_recv_msgs(struct ipmi_smi *intf);
>  static void need_waiter(struct ipmi_smi *intf);
>  static int handle_one_recv_msg(struct ipmi_smi *intf,
> @@ -498,13 +499,13 @@ struct ipmi_smi {
>   /*
>* Messages queued for delivery.  If delivery fails (out of memory
>* for instance), They will stay in here to be processed later in a
> -  * periodic timer interrupt.  The tasklet is for handling received
> +  * periodic timer interrupt.  The work is for handling received
>* messages directly from the handler.
>*/
>   spinlock_t   waiting_rcv_msgs_lock;
>   struct list_head waiting_rcv_msgs;
>   atomic_t watchdog_pretimeouts_to_deliver;
> - struct tasklet_struct recv_tasklet;
> + struct work_struct recv_work;
>  
>   spinlock_t xmit_msgs_lock;
>   struct list_head   xmit_msgs;
> @@ -704,7 +705,7 @@ static void clean_up_interface_data(struct ipmi_smi *intf)
>   struct cmd_rcvr  *rcvr, *rcvr2;
>   struct list_head list;
>  
> - tasklet_kill(>recv_tasklet);
> + cancel_work_sync(>recv_work);
>  
>   free_smi_msg_list(>waiting_rcv_msgs);
>   free_recv_msg_list(>waiting_events);
> @@ -1319,7 +1320,7 @@ static void free_user(struct kref *ref)
>  {
>   struct ipmi_user *user = container_of(ref, struct ipmi_user, refcount);
>  
> - /* SRCU cleanup must happen in task context. */
> + /* SRCU cleanup must happen in work context. */
>   queue_work(remove_work_wq, >remove_work);
>  }
>  
> @@ -3605,8 +3606,7 @@ int ipmi_add_smi(struct module *owner,
>   intf->curr_seq = 0;
>   spin_lock_init(>waiting_rcv_msgs_lock);
>   INIT_LIST_HEAD(>waiting_rcv_msgs);
> - tasklet_setup(>recv_tasklet,
> -  smi_recv_tasklet);
> + INIT_WORK(>recv_work, smi_recv_work);
>   atomic_set(>watchdog_pretimeouts_to_deliver, 0);
>   spin_lock_init(>xmit_msgs_lock);
>   INIT_LIST_HEAD(>xmit_msgs);
> @@ -4779,7 +4779,7 @@ static void handle_new_recv_msgs(struct ipmi_smi *intf)
>* To preserve message order, quit if we
>* can't handle a message.  Add the message
>* back at the head, this is safe because this
> -  * tasklet is the only thing that pulls the
> +  * work is the only thing that pulls the
>* messages.
>*/
>   list_add(_msg->link, >waiting_rcv_msgs);
> @@ -4812,10 +4812,10 @@ static void handle_new_recv_msgs(struct ipmi_smi 
> *intf)
>   }
>  }
>  
> -static void smi_recv_tasklet(struct tasklet_struct *t)
> +static void smi_recv_work(struct work_struct *t)
>  {
>   unsigned long flags = 0; /* keep us warning-free. */
> - struct ipmi_smi *intf = from_tasklet(intf, t, recv_tasklet);
> + struct ipmi_smi *intf = from_work(intf, t, recv_work);
>   int run_to_completion = intf->run_to_completion;
>   struct ipmi_smi_msg *newmsg = NULL;
>  
> @@ -4866,7 +4866,7 @@ void ipmi_smi_msg_received(struct ipmi_smi *intf,
>  
>   /*
>* To preserve message order, we keep a queue and deliver from
> -  * a tasklet.
> +  * a work.
>*/
>   if (!run_to_completion)
>   spin_lock_irqsave(>waiting_rcv_msgs_lock, flags);
> @@ -4887,9 

Re: [PATCH 21/30] panic: Introduce the panic pre-reboot notifier list

[Corey Minyard]

On Wed, Apr 27, 2022 at 07:49:15PM -0300, Guilherme G. Piccoli wrote:
> This patch renames the panic_notifier_list to panic_pre_reboot_list;
> the idea is that a subsequent patch will refactor the panic path
> in order to better split the notifiers, running some of them very
> early, some of them not so early [but still before kmsg_dump()] and
> finally, the rest should execute late, after kdump. The latter ones
> are now in the panic pre-reboot list - the name comes from the idea
> that these notifiers execute before panic() attempts rebooting the
> machine (if that option is set).
> 
> We also took the opportunity to clean-up useless header inclusions,
> improve some notifier block declarations (e.g. in ibmasm/heartbeat.c)
> and more important, change some priorities - we hereby set 2 notifiers
> to run late in the list [iss_panic_event() and the IPMI panic_event()]
> due to the risks they offer (may not return, for example).
> Proper documentation is going to be provided in a subsequent patch,
> that effectively refactors the panic path.

For the IPMI portion:

Acked-by: Corey Minyard 

Note that the IPMI panic_event() should always return, but it may take
some time, especially if the IPMI controller is no longer functional.
So the risk of a long delay is there and it makes sense to move it very
late.

-corey

> 
> Cc: Alex Elder 
> Cc: Alexander Gordeev 
> Cc: Anton Ivanov 
> Cc: Benjamin Herrenschmidt 
> Cc: Bjorn Andersson 
> Cc: Boris Ostrovsky 
> Cc: Chris Zankel 
> Cc: Christian Borntraeger 
> Cc: Corey Minyard 
> Cc: Dexuan Cui 
> Cc: "H. Peter Anvin" 
> Cc: Haiyang Zhang 
> Cc: Heiko Carstens 
> Cc: Helge Deller 
> Cc: Ivan Kokshaysky 
> Cc: "James E.J. Bottomley" 
> Cc: James Morse 
> Cc: Johannes Berg 
> Cc: Juergen Gross 
> Cc: "K. Y. Srinivasan" 
> Cc: Mathieu Poirier 
> Cc: Matt Turner 
> Cc: Mauro Carvalho Chehab 
> Cc: Max Filippov 
> Cc: Michael Ellerman 
> Cc: Paul Mackerras 
> Cc: Pavel Machek 
> Cc: Richard Henderson 
> Cc: Richard Weinberger 
> Cc: Robert Richter 
> Cc: Stefano Stabellini 
> Cc: Stephen Hemminger 
> Cc: Sven Schnelle 
> Cc: Tony Luck 
> Cc: Vasily Gorbik 
> Cc: Wei Liu 
> Signed-off-by: Guilherme G. Piccoli 
> ---
> 
> Notice that, with this name change, out-of-tree code that relies in the global
> exported "panic_notifier_list" will fail to build. We could easily keep the
> retro-compatibility by making the old symbol to still exist and point to the
> pre_reboot list (or even, keep the old naming).
> 
> But our design choice was to allow the breakage, making users rethink their
> notifiers, adding them in the list that fits best. If that wasn't a good
> decision, we're open to change it, of course.
> Thanks in advance for the review!
> 
>  arch/alpha/kernel/setup.c |  4 ++--
>  arch/parisc/kernel/pdc_chassis.c  |  3 +--
>  arch/powerpc/kernel/setup-common.c|  2 +-
>  arch/s390/kernel/ipl.c|  4 ++--
>  arch/um/drivers/mconsole_kern.c   |  2 +-
>  arch/um/kernel/um_arch.c  |  2 +-
>  arch/x86/xen/enlighten.c  |  2 +-
>  arch/xtensa/platforms/iss/setup.c |  4 ++--
>  drivers/char/ipmi/ipmi_msghandler.c   | 12 +++-
>  drivers/edac/altera_edac.c|  3 +--
>  drivers/hv/vmbus_drv.c|  4 ++--
>  drivers/leds/trigger/ledtrig-panic.c  |  3 +--
>  drivers/misc/ibmasm/heartbeat.c   | 16 +---
>  drivers/net/ipa/ipa_smp2p.c   |  5 ++---
>  drivers/parisc/power.c|  4 ++--
>  drivers/remoteproc/remoteproc_core.c  |  6 --
>  drivers/s390/char/con3215.c   |  2 +-
>  drivers/s390/char/con3270.c   |  2 +-
>  drivers/s390/char/sclp_con.c  |  2 +-
>  drivers/s390/char/sclp_vt220.c|  2 +-
>  drivers/staging/olpc_dcon/olpc_dcon.c |  6 --
>  drivers/video/fbdev/hyperv_fb.c   |  4 ++--
>  include/linux/panic_notifier.h|  2 +-
>  kernel/panic.c|  9 -
>  24 files changed, 54 insertions(+), 51 deletions(-)
> 
> diff --git a/arch/alpha/kernel/setup.c b/arch/alpha/kernel/setup.c
> index d88bdf852753..8ace0d7113b6 100644
> --- a/arch/alpha/kernel/setup.c
> +++ b/arch/alpha/kernel/setup.c
> @@ -472,8 +472,8 @@ setup_arch(char **cmdline_p)
>   }
>  
>   /* Register a call for panic conditions. */
> - atomic_notifier_chain_register(_notifier_list,
> - _panic_block);
> + atomic_notifier_chain_register(_pre_reboot_list,
> + _panic_block);
>  
>  #ifndef alpha_using_srm
>   /* Assume that we've booted from SRM if we haven't booted from

Re: [PATCH v1 1/1] kernel.h: Split out panic and oops helpers

[Corey Minyard]

On Tue, Apr 06, 2021 at 04:31:58PM +0300, Andy Shevchenko wrote:
> kernel.h is being used as a dump for all kinds of stuff for a long time.
> Here is the attempt to start cleaning it up by splitting out panic and
> oops helpers.
> 
> At the same time convert users in header and lib folder to use new header.
> Though for time being include new header back to kernel.h to avoid twisted
> indirected includes for existing users.

For the IPMI portion:

Acked-by: Corey Minyard 

> 
> Signed-off-by: Andy Shevchenko 
> ---
>  arch/powerpc/kernel/setup-common.c   |  1 +
>  arch/x86/include/asm/desc.h  |  1 +
>  arch/x86/kernel/cpu/mshyperv.c   |  1 +
>  arch/x86/kernel/setup.c  |  1 +
>  drivers/char/ipmi/ipmi_msghandler.c  |  1 +
>  drivers/remoteproc/remoteproc_core.c |  1 +
>  include/asm-generic/bug.h|  3 +-
>  include/linux/kernel.h   | 84 +---
>  include/linux/panic.h| 98 
>  include/linux/panic_notifier.h   | 12 
>  kernel/hung_task.c   |  1 +
>  kernel/kexec_core.c  |  1 +
>  kernel/panic.c   |  1 +
>  kernel/rcu/tree.c|  2 +
>  kernel/sysctl.c  |  1 +
>  kernel/trace/trace.c |  1 +
>  16 files changed, 126 insertions(+), 84 deletions(-)
>  create mode 100644 include/linux/panic.h
>  create mode 100644 include/linux/panic_notifier.h
> 
> diff --git a/arch/powerpc/kernel/setup-common.c 
> b/arch/powerpc/kernel/setup-common.c
> index 74a98fff2c2f..046fe21b5c3b 100644
> --- a/arch/powerpc/kernel/setup-common.c
> +++ b/arch/powerpc/kernel/setup-common.c
> @@ -9,6 +9,7 @@
>  #undef DEBUG
>  
>  #include 
> +#include 
>  #include 
>  #include 
>  #include 
> diff --git a/arch/x86/include/asm/desc.h b/arch/x86/include/asm/desc.h
> index 476082a83d1c..ceb12683b6d1 100644
> --- a/arch/x86/include/asm/desc.h
> +++ b/arch/x86/include/asm/desc.h
> @@ -9,6 +9,7 @@
>  #include 
>  #include 
>  
> +#include 
>  #include 
>  #include 
>  
> diff --git a/arch/x86/kernel/cpu/mshyperv.c b/arch/x86/kernel/cpu/mshyperv.c
> index 22f13343b5da..9e5c6f2b044d 100644
> --- a/arch/x86/kernel/cpu/mshyperv.c
> +++ b/arch/x86/kernel/cpu/mshyperv.c
> @@ -17,6 +17,7 @@
>  #include 
>  #include 
>  #include 
> +#include 
>  #include 
>  #include 
>  #include 
> diff --git a/arch/x86/kernel/setup.c b/arch/x86/kernel/setup.c
> index 59e5e0903b0c..570699eecf90 100644
> --- a/arch/x86/kernel/setup.c
> +++ b/arch/x86/kernel/setup.c
> @@ -14,6 +14,7 @@
>  #include 
>  #include 
>  #include 
> +#include 
>  #include 
>  #include 
>  #include 
> diff --git a/drivers/char/ipmi/ipmi_msghandler.c 
> b/drivers/char/ipmi/ipmi_msghandler.c
> index 8a0e97b33cae..e96cb5c4f97a 100644
> --- a/drivers/char/ipmi/ipmi_msghandler.c
> +++ b/drivers/char/ipmi/ipmi_msghandler.c
> @@ -16,6 +16,7 @@
>  
>  #include 
>  #include 
> +#include 
>  #include 
>  #include 
>  #include 
> diff --git a/drivers/remoteproc/remoteproc_core.c 
> b/drivers/remoteproc/remoteproc_core.c
> index 626a6b90fba2..76dd8e2b1e7e 100644
> --- a/drivers/remoteproc/remoteproc_core.c
> +++ b/drivers/remoteproc/remoteproc_core.c
> @@ -20,6 +20,7 @@
>  #include 
>  #include 
>  #include 
> +#include 
>  #include 
>  #include 
>  #include 
> diff --git a/include/asm-generic/bug.h b/include/asm-generic/bug.h
> index 76a10e0dca9f..719410b93f99 100644
> --- a/include/asm-generic/bug.h
> +++ b/include/asm-generic/bug.h
> @@ -17,7 +17,8 @@
>  #endif
>  
>  #ifndef __ASSEMBLY__
> -#include 
> +#include 
> +#include 
>  
>  #ifdef CONFIG_BUG
>  
> diff --git a/include/linux/kernel.h b/include/linux/kernel.h
> index 09035ac67d4b..6c5a05ac1ecb 100644
> --- a/include/linux/kernel.h
> +++ b/include/linux/kernel.h
> @@ -14,6 +14,7 @@
>  #include 
>  #include 
>  #include 
> +#include 
>  #include 
>  #include 
>  #include 
> @@ -70,7 +71,6 @@
>  #define lower_32_bits(n) ((u32)((n) & 0x))
>  
>  struct completion;
> -struct pt_regs;
>  struct user;
>  
>  #ifdef CONFIG_PREEMPT_VOLUNTARY
> @@ -175,14 +175,6 @@ void __might_fault(const char *file, int line);
>  static inline void might_fault(void) { }
>  #endif
>  
> -extern struct atomic_notifier_head panic_notifier_list;
> -extern long (*panic_blink)(int state);
> -__printf(1, 2)
> -void panic(const char *fmt, ...) __noreturn __cold;
> -void nmi_panic(struct pt_regs *regs, const char *msg);
> -extern void oops_enter(void);
> -extern void oops_exit(void);
> -extern bool o

Re: [PATCH -next] ipmi/powernv: Fix error return code in ipmi_powernv_probe()

[Corey Minyard]

On 01/17/2018 10:04 PM, Alexey Kardashevskiy wrote:

On 17/01/18 22:25, Wei Yongjun wrote:

Fix to return a negative error code from the request_irq() error
handling case instead of 0, as done elsewhere in this function.

Signed-off-by: Wei Yongjun 


Reviewed-by: Alexey Kardashevskiy 


Queued for next release.  Thanks!

-corey





---
  drivers/char/ipmi/ipmi_powernv.c | 5 +++--
  1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/drivers/char/ipmi/ipmi_powernv.c b/drivers/char/ipmi/ipmi_powernv.c
index c687c8d..bcf493d 100644
--- a/drivers/char/ipmi/ipmi_powernv.c
+++ b/drivers/char/ipmi/ipmi_powernv.c
@@ -250,8 +250,9 @@ static int ipmi_powernv_probe(struct platform_device *pdev)
ipmi->irq = opal_event_request(prop);
}
  
-	if (request_irq(ipmi->irq, ipmi_opal_event, IRQ_TYPE_LEVEL_HIGH,

-   "opal-ipmi", ipmi)) {
+   rc = request_irq(ipmi->irq, ipmi_opal_event, IRQ_TYPE_LEVEL_HIGH,
+"opal-ipmi", ipmi);
+   if (rc) {
dev_warn(dev, "Unable to request irq\n");
goto err_dispose;
}

Re: [PATCH -next] ipmi/powernv: Fix error return code in ipmi_powernv_probe()

[Corey Minyard]

On 01/17/2018 05:25 AM, Wei Yongjun wrote:

Fix to return a negative error code from the request_irq() error
handling case instead of 0, as done elsewhere in this function.


I think you are right here.  However, you had a bunch of people on the email
that probably didn't need to be there, and didn't have a few that should.
I've adjusted in this response.

This was introduced in change dce143c3381c355ef73be3dd97cf3ca1b15359b8,
you should add a "Fixes:" in the commit text.

I'll let the people that did this code comment, just to be sure, and 
wait for

a v2 patch from you after that.

Thanks,

-corey


Signed-off-by: Wei Yongjun 
---
  drivers/char/ipmi/ipmi_powernv.c | 5 +++--
  1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/drivers/char/ipmi/ipmi_powernv.c b/drivers/char/ipmi/ipmi_powernv.c
index c687c8d..bcf493d 100644
--- a/drivers/char/ipmi/ipmi_powernv.c
+++ b/drivers/char/ipmi/ipmi_powernv.c
@@ -250,8 +250,9 @@ static int ipmi_powernv_probe(struct platform_device *pdev)
ipmi->irq = opal_event_request(prop);
}
  
-	if (request_irq(ipmi->irq, ipmi_opal_event, IRQ_TYPE_LEVEL_HIGH,

-   "opal-ipmi", ipmi)) {
+   rc = request_irq(ipmi->irq, ipmi_opal_event, IRQ_TYPE_LEVEL_HIGH,
+"opal-ipmi", ipmi);
+   if (rc) {
dev_warn(dev, "Unable to request irq\n");
goto err_dispose;
}

Re: [-next PATCH 0/4] sysfs and DEVICE_ATTR_

[Corey Minyard]

On 12/19/2017 12:15 PM, Joe Perches wrote:

  drivers/char/ipmi/ipmi_msghandler.c| 17 +++---


For ipmi:

Acked-by: Corey Minyard <cminy...@mvista.com>

Re: [PATCH] char: ipmi: constify ipmi_smi_handlers structures

[Corey Minyard]

On 01/15/2017 01:15 PM, Bhumika Goyal wrote:

Declare ipmi_smi_handlers structures as const as they are only passed as
an argument to the function ipmi_register_smi. This argument is of type
const, so ipmi_smi_handlers structures having similar properties can be
declared const too.
Done using Coccinelle:


Thanks, queues for the next Linux patch mayhem.

-corey


@r1 disable optional_qualifier@
identifier i;
position p;
@@
static struct ipmi_smi_handlers i@p={...};

@ok1@
identifier r1.i;
position p;
@@
ipmi_register_smi(@p,...)

@bad@
position p!={r1.p,ok1.p};
identifier r1.i;
@@
i@p

@depends on !bad disable optional_qualifier@
identifier r1.i;
@@
+const
struct ipmi_smi_handlers i;

Size details after cross compiling the .o file for powerpc architecture

File size before:
   textdata bss dec hex filename
   2777 288   03065 bf9 drivers/char/ipmi/ipmi_powernv.o

File size after:
   textdata bss dec hex filename
2873192   03065 bf9 drivers/char/ipmi/ipmi_powernv.o

Signed-off-by: Bhumika Goyal 
---
  drivers/char/ipmi/ipmi_powernv.c | 2 +-
  1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/char/ipmi/ipmi_powernv.c b/drivers/char/ipmi/ipmi_powernv.c
index 6e658aa..b338a4b 100644
--- a/drivers/char/ipmi/ipmi_powernv.c
+++ b/drivers/char/ipmi/ipmi_powernv.c
@@ -196,7 +196,7 @@ static void ipmi_powernv_poll(void *send_info)
ipmi_powernv_recv(smi);
  }
  
-static struct ipmi_smi_handlers ipmi_powernv_smi_handlers = {

+static const struct ipmi_smi_handlers ipmi_powernv_smi_handlers = {
.owner  = THIS_MODULE,
.start_processing   = ipmi_powernv_start_processing,
.sender = ipmi_powernv_send,

Re: [Openipmi-developer] [PATCH 3/4] ipmi: allow dynamic BMC version information

[Corey Minyard]

On 09/12/2016 03:55 AM, Jeremy Kerr wrote:

Currently, it's up to the IPMI SMIs to provide the product & version
details of BMCs behind registered IPMI SMI interfaces. This device ID is
provided on SMI regsitration, and kept around for all future queries.

However, this version information isn't always static. For example, a
BMC may be upgraded at runtime, making the old version information
stale.

This change allows querying the BMC device ID & version information
dynamically. If no static device_id argument is provided to
ipmi_register_smi, then the IPMI core code will perform a Get Device ID
IPMI command to query the version information when needed. We keep a
short-term cache of this information so we don't need to re-query
for every attribute access.


In all, this looks good.  I have two minor nits inline below, and
two more major comments here:

I would prefer if it always queried the data, even if the device id
information is provided by the lower level driver.  The other
two interfaces query the device id for blacklisting and interface
detection, so they already have it available.  If the dev_id is
provided, you can just set the information valid and set the last
query time.  Then the disabled state is no longer necessary.

Since the values can change while you are querying them, do
we need some sort of mutex on them?  I know the chances are
pretty remote that you would ever hit an issue, but it's at least
theoretically possible.

Thanks,

-corey


Signed-off-by: Jeremy Kerr 
---
  drivers/char/ipmi/ipmi_msghandler.c | 158 ++--
  1 file changed, 153 insertions(+), 5 deletions(-)

diff --git a/drivers/char/ipmi/ipmi_msghandler.c 
b/drivers/char/ipmi/ipmi_msghandler.c
index 41990bb..55feba0 100644
--- a/drivers/char/ipmi/ipmi_msghandler.c
+++ b/drivers/char/ipmi/ipmi_msghandler.c
@@ -90,6 +90,9 @@ static struct proc_dir_entry *proc_ipmi_root;
   */
  #define IPMI_REQUEST_EV_TIME  (1000 / (IPMI_TIMEOUT_TIME))
  
+/* How long should we cache dynamic device IDs? */

+#define IPMI_DYN_DEV_ID_EXPIRY (10 * HZ)
+
  /*
   * The main "user" data structure.
   */
@@ -192,10 +195,19 @@ struct ipmi_proc_entry {
  };
  #endif
  
+enum bmc_dyn_device_id_state {

+   BMC_DEVICE_DYN_ID_STATE_DISABLED,
+   BMC_DEVICE_DYN_ID_STATE_QUERYING,
+   BMC_DEVICE_DYN_ID_STATE_VALID,
+   BMC_DEVICE_DYN_ID_STATE_INVALID,
+};
+
  struct bmc_device {
struct platform_device pdev;
struct ipmi_device_id  id;
ipmi_smi_t intf;
+   intdyn_id;
+   unsigned long  dyn_id_expiry;
unsigned char  guid[16];
intguid_set;
char   name[16];
@@ -1997,6 +2009,108 @@ int ipmi_request_supply_msgs(ipmi_user_t  user,
  }
  EXPORT_SYMBOL(ipmi_request_supply_msgs);
  
+static void bmc_device_id_handler(ipmi_smi_t intf, struct ipmi_recv_msg *msg)

+{
+   int rc;
+
+   if ((msg->addr.addr_type != IPMI_SYSTEM_INTERFACE_ADDR_TYPE)
+   || (msg->msg.netfn != IPMI_NETFN_APP_RESPONSE)
+   || (msg->msg.cmd != IPMI_GET_DEVICE_ID_CMD))
+   return;
+
+   /* Do we have a success completion code? */
+   if (msg->msg.data[0] != 0) {
+   intf->bmc->dyn_id = BMC_DEVICE_DYN_ID_STATE_INVALID;
+   goto out;
+   }
+
+   /* Do we have enough data to parse the device ID details? This doesn't
+* inclde the optional auxilliary version data. */


Minor nit: include is misspelled.


+   if (msg->msg.data_len < 12) {
+   intf->bmc->dyn_id = BMC_DEVICE_DYN_ID_STATE_INVALID;
+   goto out;
+   }
+
+   rc = ipmi_demangle_device_id(msg->msg.netfn, msg->msg.cmd,
+   msg->msg.data, msg->msg.data_len, >bmc->id);
+   if (rc) {
+   intf->bmc->dyn_id = BMC_DEVICE_DYN_ID_STATE_INVALID;
+   goto out;
+   }
+
+   intf->ipmi_version_major = ipmi_version_major(>bmc->id);
+   intf->ipmi_version_minor = ipmi_version_minor(>bmc->id);
+
+   /* All good! mark the dynamic ID as valid, and set its expiration
+* time */
+   intf->bmc->dyn_id = BMC_DEVICE_DYN_ID_STATE_VALID;
+   intf->bmc->dyn_id_expiry = jiffies + IPMI_DYN_DEV_ID_EXPIRY;
+out:
+   wake_up(>waitq);
+}
+
+
+static int bmc_update_device_id(struct bmc_device *bmc)
+{
+   struct ipmi_system_interface_addr si;
+   ipmi_smi_t intf = bmc->intf;
+   struct kernel_ipmi_msg msg;
+   int rc;
+
+   if (bmc->dyn_id == BMC_DEVICE_DYN_ID_STATE_DISABLED)
+   return 0;
+
+   if (bmc->dyn_id == BMC_DEVICE_DYN_ID_STATE_VALID &&
+   time_is_after_jiffies(bmc->dyn_id_expiry))
+   return 0;
+
+   /* do we have a request in progress? Just wait for that. */
+   if (bmc->dyn_id == BMC_DEVICE_DYN_ID_STATE_QUERYING)
+   return 

Re: [PATCH] ipmi/powernv: Fix potential invalid pointer dereference

[Corey Minyard]

Ok, this looks fine.  A couple of question...

Do I need to send this upstream right now?  How well has this been tested?

Do you want this backported to 4.0 stable?

-corey

On 07/16/2015 06:16 AM, Neelesh Gupta wrote:
 If the OPAL call to receive the ipmi message fails, then we free up the
 smi message and return. But, the driver still holds the reference to
 old smi message in the 'cur_msg' which can potentially be accessed later
 and freed again leading to kernel oops. To fix it up,

 The kernel driver should reset the 'cur_msg' and send reply to the user
 in addition to freeing the message.

 Signed-off-by: Neelesh Gupta neele...@linux.vnet.ibm.com
 ---
  drivers/char/ipmi/ipmi_powernv.c |   13 ++---
  1 file changed, 10 insertions(+), 3 deletions(-)

 diff --git a/drivers/char/ipmi/ipmi_powernv.c 
 b/drivers/char/ipmi/ipmi_powernv.c
 index 9b409c0..637486d 100644
 --- a/drivers/char/ipmi/ipmi_powernv.c
 +++ b/drivers/char/ipmi/ipmi_powernv.c
 @@ -143,9 +143,16 @@ static int ipmi_powernv_recv(struct ipmi_smi_powernv 
 *smi)
   pr_devel(%s:   - %d (size %lld)\n, __func__,
   rc, rc == 0 ? size : 0);
   if (rc) {
 - spin_unlock_irqrestore(smi-msg_lock, flags);
 - ipmi_free_smi_msg(msg);
 - return 0;
 + /* If came via the poll, and response was not yet ready */
 + if (rc == OPAL_EMPTY) {
 + spin_unlock_irqrestore(smi-msg_lock, flags);
 + return 0;
 + } else {
 + smi-cur_msg = NULL;
 + spin_unlock_irqrestore(smi-msg_lock, flags);
 + send_error_reply(smi, msg, IPMI_ERR_UNSPECIFIED);
 + return 0;
 + }
   }
  
   if (size  sizeof(*opal_msg)) {


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

Re: [PATCH v3 2/8] ipmi/powernv: Convert to irq event interface

[Corey Minyard]

On 05/06/2015 10:16 PM, Alistair Popple wrote:
 Convert the opal ipmi driver to use the new irq interface for events.

 Signed-off-by: Alistair Popple alist...@popple.id.au
 Cc: Corey Minyard miny...@acm.org
 Cc: openipmi-develo...@lists.sourceforge.net
 ---

 Corey,

 If this looks ok can you please ack it? Michael Ellerman will then take
 the whole series via the powerpc tree. Thanks.

This looks fine; I don't really understand much of this, but I don't see
any issues.

The only thing I would suggest is passing the irq level
(IRQ_TYPE_LEVEL_HIGH) as
part of the openfirmware data instead of hard-coding it.

Acked-by: Corey Minyard cminy...@mvista.com


  drivers/char/ipmi/ipmi_powernv.c | 39 ++-
  1 file changed, 22 insertions(+), 17 deletions(-)

 diff --git a/drivers/char/ipmi/ipmi_powernv.c 
 b/drivers/char/ipmi/ipmi_powernv.c
 index 8753b0f..9b409c0 100644
 --- a/drivers/char/ipmi/ipmi_powernv.c
 +++ b/drivers/char/ipmi/ipmi_powernv.c
 @@ -15,6 +15,8 @@
  #include linux/list.h
  #include linux/module.h
  #include linux/of.h
 +#include linux/of_irq.h
 +#include linux/interrupt.h

  #include asm/opal.h

 @@ -23,8 +25,7 @@ struct ipmi_smi_powernv {
   u64 interface_id;
   struct ipmi_device_id   ipmi_id;
   ipmi_smi_t  intf;
 - u64 event;
 - struct notifier_block   event_nb;
 + unsigned intirq;

   /**
* We assume that there can only be one outstanding request, so
 @@ -197,15 +198,12 @@ static struct ipmi_smi_handlers 
 ipmi_powernv_smi_handlers = {
   .poll   = ipmi_powernv_poll,
  };

 -static int ipmi_opal_event(struct notifier_block *nb,
 -   unsigned long events, void *change)
 +static irqreturn_t ipmi_opal_event(int irq, void *data)
  {
 - struct ipmi_smi_powernv *smi = container_of(nb,
 - struct ipmi_smi_powernv, event_nb);
 + struct ipmi_smi_powernv *smi = data;

 - if (events  smi-event)
 - ipmi_powernv_recv(smi);
 - return 0;
 + ipmi_powernv_recv(smi);
 + return IRQ_HANDLED;
  }

  static int ipmi_powernv_probe(struct platform_device *pdev)
 @@ -240,13 +238,16 @@ static int ipmi_powernv_probe(struct platform_device 
 *pdev)
   goto err_free;
   }

 - ipmi-event = 1ull  prop;
 - ipmi-event_nb.notifier_call = ipmi_opal_event;
 + ipmi-irq = irq_of_parse_and_map(dev-of_node, 0);
 + if (!ipmi-irq) {
 + dev_info(dev, Unable to map irq from device tree\n);
 + ipmi-irq = opal_event_request(prop);
 + }

 - rc = opal_notifier_register(ipmi-event_nb);
 - if (rc) {
 - dev_warn(dev, OPAL notifier registration failed (%d)\n, rc);
 - goto err_free;
 + if (request_irq(ipmi-irq, ipmi_opal_event, IRQ_TYPE_LEVEL_HIGH,
 + opal-ipmi, ipmi)) {
 + dev_warn(dev, Unable to request irq\n);
 + goto err_dispose;
   }

   ipmi-opal_msg = devm_kmalloc(dev,
 @@ -271,7 +272,9 @@ static int ipmi_powernv_probe(struct platform_device 
 *pdev)
  err_free_msg:
   devm_kfree(dev, ipmi-opal_msg);
  err_unregister:
 - opal_notifier_unregister(ipmi-event_nb);
 + free_irq(ipmi-irq, ipmi);
 +err_dispose:
 + irq_dispose_mapping(ipmi-irq);
  err_free:
   devm_kfree(dev, ipmi);
   return rc;
 @@ -282,7 +285,9 @@ static int ipmi_powernv_remove(struct platform_device 
 *pdev)
   struct ipmi_smi_powernv *smi = dev_get_drvdata(pdev-dev);

   ipmi_unregister_smi(smi-intf);
 - opal_notifier_unregister(smi-event_nb);
 + free_irq(smi-irq, smi);
 + irq_dispose_mapping(smi-irq);
 +
   return 0;
  }

 --
 1.8.3.2


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

Re: [PATCH v2] drivers/char/ipmi: Add powernv IPMI driver

[Corey Minyard]

This looks good.  Can this go into the IPMI tree now, or does it need
work done in the PowerPC tree first?

Thanks,

-corey

On 11/12/2014 01:41 AM, Jeremy Kerr wrote:
 This change adds an initial IPMI driver for powerpc OPAL firmware. The
 interface is exposed entirely through firmware: we have two functions to
 send and receive IPMI messages, and an interrupt notification from the
 firmware to signify that a message is available.

 Signed-off-by: Jeremy Kerr j...@ozlabs.org

 ---
 v2: Update for ipmi/for-next tree, add copyright header

 ---
  drivers/char/ipmi/Kconfig|6 
  drivers/char/ipmi/Makefile   |1 
  drivers/char/ipmi/ipmi_powernv.c |  307 +++
  3 files changed, 314 insertions(+)

 diff --git a/drivers/char/ipmi/Kconfig b/drivers/char/ipmi/Kconfig
 index c1fccf4..65fb008 100644
 --- a/drivers/char/ipmi/Kconfig
 +++ b/drivers/char/ipmi/Kconfig
 @@ -72,6 +72,12 @@ config IPMI_SSIF
have a driver that must be accessed over an I2C bus instead of a
standard interface.  This module requires I2C support.
  
 +config IPMI_POWERNV
 +   depends on PPC_POWERNV
 +   tristate 'POWERNV (OPAL firmware) IPMI interface'
 +   help
 + Provides a driver for OPAL firmware-based IPMI interfaces.
 +
  config IPMI_WATCHDOG
 tristate 'IPMI Watchdog Timer'
 help
 diff --git a/drivers/char/ipmi/Makefile b/drivers/char/ipmi/Makefile
 index 115c08d..f3ffde1 100644
 --- a/drivers/char/ipmi/Makefile
 +++ b/drivers/char/ipmi/Makefile
 @@ -8,5 +8,6 @@ obj-$(CONFIG_IPMI_HANDLER) += ipmi_msghandler.o
  obj-$(CONFIG_IPMI_DEVICE_INTERFACE) += ipmi_devintf.o
  obj-$(CONFIG_IPMI_SI) += ipmi_si.o
  obj-$(CONFIG_IPMI_SSIF) += ipmi_ssif.o
 +obj-$(CONFIG_IPMI_POWERNV) += ipmi_powernv.o
  obj-$(CONFIG_IPMI_WATCHDOG) += ipmi_watchdog.o
  obj-$(CONFIG_IPMI_POWEROFF) += ipmi_poweroff.o
 diff --git a/drivers/char/ipmi/ipmi_powernv.c 
 b/drivers/char/ipmi/ipmi_powernv.c
 new file mode 100644
 index 000..50134ec
 --- /dev/null
 +++ b/drivers/char/ipmi/ipmi_powernv.c
 @@ -0,0 +1,307 @@
 +/*
 + * PowerNV OPAL IPMI driver
 + *
 + * Copyright 2014 IBM Corp.
 + *
 + * 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.
 + */
 +
 +#define pr_fmt(fmt)ipmi-powernv:  fmt
 +
 +#include linux/ipmi_smi.h
 +#include linux/list.h
 +#include linux/module.h
 +#include linux/of.h
 +
 +#include asm/opal.h
 +
 +
 +struct ipmi_smi_powernv {
 + u64 interface_id;
 + struct ipmi_device_id   ipmi_id;
 + ipmi_smi_t  intf;
 + u64 event;
 + struct notifier_block   event_nb;
 +
 + /**
 +  * We assume that there can only be one outstanding request, so
 +  * keep the pending message in cur_msg. We protect this from concurrent
 +  * updates through send  recv calls, (and consequently opal_msg, which
 +  * is in-use when cur_msg is set) with msg_lock
 +  */
 + spinlock_t  msg_lock;
 + struct ipmi_smi_msg *cur_msg;
 + struct opal_ipmi_msg*opal_msg;
 +};
 +
 +static int ipmi_powernv_start_processing(void *send_info, ipmi_smi_t intf)
 +{
 + struct ipmi_smi_powernv *smi = send_info;
 + smi-intf = intf;
 + return 0;
 +}
 +
 +static void send_error_reply(struct ipmi_smi_powernv *smi,
 + struct ipmi_smi_msg *msg, u8 completion_code)
 +{
 + msg-rsp[0] = msg-data[0] | 0x4;
 + msg-rsp[1] = msg-data[1];
 + msg-rsp[2] = completion_code;
 + msg-rsp_size = 3;
 + ipmi_smi_msg_received(smi-intf, msg);
 +}
 +
 +static void ipmi_powernv_send(void *send_info, struct ipmi_smi_msg *msg)
 +{
 + struct ipmi_smi_powernv *smi = send_info;
 + struct opal_ipmi_msg *opal_msg;
 + unsigned long flags;
 + int comp, rc;
 + size_t size;
 +
 + /* ensure data_len will fit in the opal_ipmi_msg buffer... */
 + if (msg-data_size  IPMI_MAX_MSG_LENGTH) {
 + comp = IPMI_REQ_LEN_EXCEEDED_ERR;
 + goto err;
 + }
 +
 + /* ... and that we at least have netfn and cmd bytes */
 + if (msg-data_size  2) {
 + comp = IPMI_REQ_LEN_INVALID_ERR;
 + goto err;
 + }
 +
 + spin_lock_irqsave(smi-msg_lock, flags);
 +
 + if (smi-cur_msg) {
 + comp = IPMI_NODE_BUSY_ERR;
 + goto err_unlock;
 + }
 +
 + /* format our data for the OPAL API */
 + opal_msg = smi-opal_msg;
 + opal_msg-version = OPAL_IPMI_MSG_FORMAT_VERSION_1;
 + opal_msg-netfn = msg-data[0];
 + opal_msg-cmd = msg-data[1];
 + if (msg-data_size  2)
 + memcpy(opal_msg-data, msg-data + 2, msg-data_size - 2);
 +
 + /* data_size already includes the netfn and cmd bytes */
 + size = sizeof(*opal_msg) + msg-data_size - 2;
 +
 + 

[PATCH] powerpc: Add coherent_dma_mask to mv64x60 devices

[Corey Minyard]

From: Corey Minyard cminy...@mvista.com

DMA ops requires that coherent_dma_mask be set properly for a device,
but this was not being done for devices on the MV64x60 that use DMA.
Both the serial and ethernet devices need this or they won't be able
to allocate memory.

Signed-off-by: Corey Minyard cminy...@mvista.com
---
Mark Greer pointed me to the right place, I believe this is the
correct way to handle the problem.

Index: linux-2.6/arch/powerpc/sysdev/mv64x60_dev.c
===
--- linux-2.6.orig/arch/powerpc/sysdev/mv64x60_dev.c
+++ linux-2.6/arch/powerpc/sysdev/mv64x60_dev.c
@@ -16,6 +16,7 @@
 #include linux/mv643xx.h
 #include linux/platform_device.h
 #include linux/of_platform.h
+#include linux/dma-mapping.h
 
 #include asm/prom.h
 
@@ -189,6 +190,7 @@ static int __init mv64x60_mpsc_device_se
pdev = platform_device_alloc(MPSC_CTLR_NAME, port_number);
if (!pdev)
return -ENOMEM;
+   pdev-dev.coherent_dma_mask = DMA_BIT_MASK(32);
 
err = platform_device_add_resources(pdev, r, 5);
if (err)
@@ -302,6 +304,7 @@ static int __init mv64x60_eth_device_set
if (!pdev)
return -ENOMEM;
 
+   pdev-dev.coherent_dma_mask = DMA_BIT_MASK(32);
err = platform_device_add_resources(pdev, r, 1);
if (err)
goto error;
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev

Re: [PATCH] powerpc: Add DMA mask to MPSC serial and network and UART device to serial

[Corey Minyard]

Benjamin Herrenschmidt wrote:

On Fri, 2010-01-29 at 18:04 -0600, Corey Minyard wrote:
  

From: Corey Minyard cminy...@mvista.com

The MPSC drivers that use DMA need to set coherent_dma_mask to allow
dma_alloc_xxx routines to work properly.  Also, the mpsc serial driver
needed to set pi-port.dev to register properly.  With these fixes,
the MPSC drivers seem to work again.

Signed-off-by: Corey Minyard cminy...@mvista.com
---



Is that enough ? Since 2.6.31 we also need the dma ops to be filled
properly and obviously that isn't going to happen for random platform
devices... Or are those initialized elsewhere by your platform code ? In
which case it might be a good place to also set the coherent mask...
  

That's done in ppc_dflt_bus_notify(), but that didn't seem an appropriate
place to do this.  If it is, it's easy enough to add it there, but that 
would

mean it would get set for all devices on any type of ppc system.

-corey

Maybe we should have a more generic way to set the default dma
information (including ops, masks etc...) for use by platform,
of_platform etc... devices.

Cheers,
Ben.

  

I'm not really sure about where to set the coherent_dma_mask, it seems
like a more general place would be better but I couldn't find it.

Without these, the console stops working after the switchover andantipode
the network driver fails to allocate buffers.  With these, my board
boots up and works fine.

Index: linux-2.6.31/drivers/serial/mpsc.c
===
--- linux-2.6.31.orig/drivers/serial/mpsc.c
+++ linux-2.6.31/drivers/serial/mpsc.c
@@ -2071,6 +2071,9 @@ static int mpsc_drv_probe(struct platfor
if (!(rc = mpsc_drv_map_regs(pi, dev))) {
mpsc_drv_get_platform_data(pi, dev, dev-id);
 
+			dev-dev.coherent_dma_mask = DMA_BIT_MASK(32);

+   pi-port.dev = dev-dev;
+
if (!(rc = mpsc_make_ready(pi))) {
spin_lock_init(pi-tx_lock);
if (!(rc = uart_add_one_port(mpsc_reg,
Index: linux-2.6.31/drivers/net/mv643xx_eth.c
===
--- linux-2.6.31.orig/drivers/net/mv643xx_eth.c
+++ linux-2.6.31/drivers/net/mv643xx_eth.c
@@ -2916,7 +2916,7 @@ static int mv643xx_eth_probe(struct plat
mp-shared = platform_get_drvdata(pd-shared);
mp-base = mp-shared-base + 0x0400 + (pd-port_number  10);
mp-port_num = pd-port_number;
-
+   pdev-dev.coherent_dma_mask = DMA_BIT_MASK(32);
mp-dev = dev;
 
 	set_params(mp, pd);

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




  


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

[PATCH] powerpc: Set the port device in the mpsc serial driver

[Corey Minyard]

From: Corey Minyard cminy...@mvista.com

The mpsc serial driver needx to set the port's device tree element
to register properly.

Signed-off-by: Corey Minyard cminy...@mvista.com
---

Index: linux-2.6/drivers/serial/mpsc.c
===
--- linux-2.6.orig/drivers/serial/mpsc.c
+++ linux-2.6/drivers/serial/mpsc.c
@@ -2070,6 +2070,7 @@ static int mpsc_drv_probe(struct platfor
 
if (!(rc = mpsc_drv_map_regs(pi, dev))) {
mpsc_drv_get_platform_data(pi, dev, dev-id);
+   pi-port.dev = dev-dev;
 
if (!(rc = mpsc_make_ready(pi))) {
spin_lock_init(pi-tx_lock);
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev

[PATCH] powerpc: Add coherent_dma_mask setting to platform devices

[Corey Minyard]

From: Corey Minyard cminy...@mvista.com

DMA ops requires that coherent_dma_mask be set properly for a device,
but this was not being done for platform devices on powerpc.  The
MPSC drivers, in particular, need this for both serial and ethernet
or they won't be able to allocate memory.

Signed-off-by: Corey Minyard cminy...@mvista.com
---
How about this patch?  It seems to work ok and I suppose this makes
sense.  I'll send the uart setting in another patch.

Index: linux-2.6/arch/powerpc/kernel/setup-common.c
===
--- linux-2.6.orig/arch/powerpc/kernel/setup-common.c
+++ linux-2.6/arch/powerpc/kernel/setup-common.c
@@ -681,6 +681,7 @@ static int ppc_dflt_bus_notify(struct no
return 0;
 
set_dma_ops(dev, dma_direct_ops);
+   pdev-dev.coherent_dma_mask = DMA_BIT_MASK(32);
 
return NOTIFY_DONE;
 }
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev

[PATCH] powerpc: Add a new platform for maple so a different link address can be set

[Corey Minyard]

From: Corey Minyard cminy...@mvista.com

The maple platform failed to load because it's firmware could not take a
link address of 0x400.  A new platform type with a link address of
0x40 had to be created for the maple.

Signed-off-by: Corey Minyard cminy...@mvista.com
---
Without this patch the firmware loader says it is unable to parse the
ELF header and dies.  This patch lets 2.6.31 boot without issue.  At
the current head of k.org git, at least with an NFS mount, I'm getting:
  Kernel panic - not syncing: No init found
So I'm not sure what the deal is there, but this patch is still required
to get the firmware to load the kernel.

Index: linux-2.6.31/arch/powerpc/boot/wrapper
===
--- linux-2.6.31.orig/arch/powerpc/boot/wrapper
+++ linux-2.6.31/arch/powerpc/boot/wrapper
@@ -145,6 +145,10 @@ pseries)
 platformo=$object/of.o
 link_address='0x400'
 ;;
+maple)
+platformo=$object/of.o
+link_address='0x40'
+;;
 pmac|chrp)
 platformo=$object/of.o
 ;;
@@ -313,7 +319,7 @@ fi
 
 # post-processing needed for some platforms
 case $platform in
-pseries|chrp)
+pseries|chrp|maple)
 $objbin/addnote $ofile
 ;;
 coff)
Index: linux-2.6.31/arch/powerpc/boot/Makefile
===
--- linux-2.6.31.orig/arch/powerpc/boot/Makefile
+++ linux-2.6.31/arch/powerpc/boot/Makefile
@@ -167,7 +167,7 @@ quiet_cmd_wrap  = WRAP$@
$(if $3, -s $3)$(if $4, -d $4)$(if $5, -i $5) vmlinux
 
 image-$(CONFIG_PPC_PSERIES)+= zImage.pseries
-image-$(CONFIG_PPC_MAPLE)  += zImage.pseries
+image-$(CONFIG_PPC_MAPLE)  += zImage.maple
 image-$(CONFIG_PPC_IBM_CELL_BLADE) += zImage.pseries
 image-$(CONFIG_PPC_PS3)+= dtbImage.ps3
 image-$(CONFIG_PPC_CELLEB) += zImage.pseries
@@ -346,7 +346,7 @@ install: $(CONFIGURE) $(addprefix $(obj)
 clean-files += $(image-) $(initrd-) cuImage.* dtbImage.* treeImage.* \
zImage zImage.initrd zImage.chrp zImage.coff zImage.holly \
zImage.iseries zImage.miboot zImage.pmac zImage.pseries \
-   simpleImage.* otheros.bld *.dtb
+   zImage.maple simpleImage.* otheros.bld *.dtb
 
 # clean up files cached by wrapper
 clean-kernel := vmlinux.strip vmlinux.bin
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev

[PATCH] powerpc: Add DMA mask to MPSC serial and network and UART device to serial

[Corey Minyard]

From: Corey Minyard cminy...@mvista.com

The MPSC drivers that use DMA need to set coherent_dma_mask to allow
dma_alloc_xxx routines to work properly.  Also, the mpsc serial driver
needed to set pi-port.dev to register properly.  With these fixes,
the MPSC drivers seem to work again.

Signed-off-by: Corey Minyard cminy...@mvista.com
---
I'm not really sure about where to set the coherent_dma_mask, it seems
like a more general place would be better but I couldn't find it.

Without these, the console stops working after the switchover and
the network driver fails to allocate buffers.  With these, my board
boots up and works fine.

Index: linux-2.6.31/drivers/serial/mpsc.c
===
--- linux-2.6.31.orig/drivers/serial/mpsc.c
+++ linux-2.6.31/drivers/serial/mpsc.c
@@ -2071,6 +2071,9 @@ static int mpsc_drv_probe(struct platfor
if (!(rc = mpsc_drv_map_regs(pi, dev))) {
mpsc_drv_get_platform_data(pi, dev, dev-id);
 
+   dev-dev.coherent_dma_mask = DMA_BIT_MASK(32);
+   pi-port.dev = dev-dev;
+
if (!(rc = mpsc_make_ready(pi))) {
spin_lock_init(pi-tx_lock);
if (!(rc = uart_add_one_port(mpsc_reg,
Index: linux-2.6.31/drivers/net/mv643xx_eth.c
===
--- linux-2.6.31.orig/drivers/net/mv643xx_eth.c
+++ linux-2.6.31/drivers/net/mv643xx_eth.c
@@ -2916,7 +2916,7 @@ static int mv643xx_eth_probe(struct plat
mp-shared = platform_get_drvdata(pd-shared);
mp-base = mp-shared-base + 0x0400 + (pd-port_number  10);
mp-port_num = pd-port_number;
-
+   pdev-dev.coherent_dma_mask = DMA_BIT_MASK(32);
mp-dev = dev;
 
set_params(mp, pd);
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev