linux-kernel - RE: [PATCH V5 1/2] irqchip: imx-gpcv2: IMX GPCv2 driver for wakeup sources

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Date:	Tue, 21 Jul 2015 23:37:34 +0200 (CEST)
From:	Thomas Gleixner <tglx@...utronix.de>
To:	Shenwei Wang <Shenwei.Wang@...escale.com>
cc:	"shawn.guo@...aro.org" <shawn.guo@...aro.org>,
	"jason@...edaemon.net" <jason@...edaemon.net>,
	"linux-arm-kernel@...ts.infradead.org" 
	<linux-arm-kernel@...ts.infradead.org>,
	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>
Subject: RE: [PATCH V5 1/2] irqchip: imx-gpcv2: IMX GPCv2 driver for wakeup
 sources

On Tue, 21 Jul 2015, Shenwei Wang wrote:
> > On Tue, 21 Jul 2015, Shenwei Wang wrote:
> > > This struct defines the properties and functions that GPCv2 block
> > > provides. Since GPCv2 has two key functions: Irq wakeup source
> > > management and power management, the intention of the struct is to
> > > share data and methods among irqchip, suspend, and cpuidle drivers.
> > 
> > I don't think this is a good idea. The cpuidle driver has nothing to know about the
> > internals of the irq driver and vice versa. Neither does the suspend code.
> > 
> > If you failed to split that proper then your design is wrong.
> > 
> The implementation has already been spitted totally. The question is
> if we use the same structure among those drivers or not, since they
> do share some common data like gpc_base address, enabled_irq, and
> mfmix_mask. The suspend and cpuidle driver will use those data to
> decide the hardware power modes and the relating power down sequence
> of the power domains. The structure is the abstract of the GPCv2
> hardware, and the current struct declaration matches the low level
> hardware well. Although it is possible and easy to split it into
> two, it may introduce either redundant definition for the common
> properties or have to create a global variable to enable them
> visible to both the irqchip and the suspend codes.

So the proper way to do this is:

Have a data structure which contains only the shared information. The
pointer to this structure can be global.

Have per driver data structures which contain the driver private
stuff.

Think about whether you need all the function pointers in one of the
structs. IOW, you need them only if a pointer can be changed at
runtime. If not you can call the function directly as I doubt that any
of these drivers can be modular.

Thanks,

	tglx




--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/