From mboxrd@z Thu Jan 1 00:00:00 1970 From: Sasha Levin Subject: Re: [PATCH v2 1/8] kvm tools: Add optional parameter used in ioport callbacks Date: Thu, 26 May 2011 12:14:29 +0300 Message-ID: <1306401269.3065.12.camel@lappy> References: <1306392135-16993-1-git-send-email-levinsasha928@gmail.com> <1306400009.16757.118.camel@jaguar> <1306400568.3065.8.camel@lappy> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Cc: john@jfloren.net, kvm@vger.kernel.org, mingo@elte.hu, asias.hejun@gmail.com, gorcunov@gmail.com, prasadjoshi124@gmail.com To: Pekka Enberg Return-path: Received: from mail-wy0-f174.google.com ([74.125.82.174]:44352 "EHLO mail-wy0-f174.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1753703Ab1EZJOr (ORCPT ); Thu, 26 May 2011 05:14:47 -0400 Received: by wya21 with SMTP id 21so333957wya.19 for ; Thu, 26 May 2011 02:14:46 -0700 (PDT) In-Reply-To: Sender: kvm-owner@vger.kernel.org List-ID: On Thu, 2011-05-26 at 12:04 +0300, Pekka Enberg wrote: > On Thu, May 26, 2011 at 12:02 PM, Sasha Levin wrote: > > On Thu, 2011-05-26 at 11:53 +0300, Pekka Enberg wrote: > >> On Thu, 2011-05-26 at 09:42 +0300, Sasha Levin wrote: > >> > Allow specifying an optional parameter when registering an > >> > ioport range. The callback functions provided by the registering > >> > module will be called with the same parameter. > >> > > >> > This may be used to keep context during callbacks on IO operations. > >> > > >> > Signed-off-by: Sasha Levin > >> > --- > >> > tools/kvm/include/kvm/ioport.h | 3 ++ > >> > tools/kvm/ioport.c | 54 +++++++++++++++++++++++++++++---------- > >> > 2 files changed, 43 insertions(+), 14 deletions(-) > >> > > >> > diff --git a/tools/kvm/include/kvm/ioport.h b/tools/kvm/include/kvm/ioport.h > >> > index 8253938..2a8d74d 100644 > >> > --- a/tools/kvm/include/kvm/ioport.h > >> > +++ b/tools/kvm/include/kvm/ioport.h > >> > @@ -25,11 +25,14 @@ struct kvm; > >> > struct ioport_operations { > >> > bool (*io_in)(struct kvm *kvm, u16 port, void *data, int size, u32 count); > >> > bool (*io_out)(struct kvm *kvm, u16 port, void *data, int size, u32 count); > >> > + bool (*io_in_param)(struct kvm *kvm, u16 port, void *data, int size, u32 count, void *param); > >> > + bool (*io_out_param)(struct kvm *kvm, u16 port, void *data, int size, u32 count, void *param); > >> > >> So why not make that 'param' unconditional for io_in and io_out and just > >> pass NULL if it's not needed? > >> > > > > I've wanted to keep the original interface clean, Most of the IO port > > users don't (and probably won't) require a parameter. > > Well now struct ioport_operations isn't very clean is it - or the code > that needs to determine which function pointer to call?-) struct ioport_operations is a bit more messy, but it's one spot instead of adding a 'parameter' to each module that doesn't really need it. My assumption is that most ioport users now and in the future won't need it, it just solves several special cases more easily (multiple devices which share same handling functions). -- Sasha.