linux-kernel - Re: [PATCH 4/8] minnowboard: Add base platform driver for the MinnowBoard

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] [thread-next>] [day] [month] [year] [list]

Date:	Thu, 27 Jun 2013 22:43:38 -0700
From:	Darren Hart <dvhart@...ux.intel.com>
To:	Linus Walleij <linus.walleij@...aro.org>
Cc:	Linux Kernel Mailing List <linux-kernel@...r.kernel.org>,
	"H. Peter Anvin" <hpa@...or.com>,
	"peter.p.waskiewicz.jr" <peter.p.waskiewicz.jr@...el.com>,
	Andy Shevchenko <andriy.shevchenko@...ux.intel.com>,
	danders <danders@...cuitco.com>,
	"vishal.l.verma" <vishal.l.verma@...el.com>,
	Matthew Garrett <matthew.garrett@...ula.com>,
	Grant Likely <grant.likely@...aro.org>,
	Richard Purdie <richard.purdie@...uxfoundation.org>,
	platform-driver-x86 <platform-driver-x86@...r.kernel.org>
Subject: Re: [PATCH 4/8] minnowboard: Add base platform driver for the
 MinnowBoard

On Thu, 2013-06-27 at 11:14 +0200, Linus Walleij wrote:
> On Wed, Jun 26, 2013 at 3:53 AM, Darren Hart <dvhart@...ux.intel.com> wrote:
> 
> > Provide a minimal public interface:
> >         minnow_detect()
> >         minnow_lvds_detect()
> >         minnow_hwid()
> >         minnow_phy_reset()
> 
> So instead of these calling drivers issueing gpio_request() themselves
> to obtain a resource, they make a function call to this proxy that issue
> gpio_request() for them.
> 
> This is generally not how we do things. A driver should request its
> GPIO just as it requests its regulator or clock or IRQ line or anything
> else.

I'll fix this for minnow_phy_reset() by moving the reset routine into
the pch_gbe driver and have it request the GPIO, using the pci_id
structure to determine the GPIO line.

I'll drop minnow_lvds_detect() and work toward the firmware managing
this aspect instead.

minnow_detect() doesn't access GPIO.

minnow_hwid() just returns an int that the minnowboard platform driver
read from the GPIO. This seems like a proper abstraction to me. Do you
object to this one as well?

>  Centralizing resource handling is not a good idea IMO, it's better
> that each driver request it's GPIO pin(s) and do the stuff it needs
> with them.

Understood. Since minnow_hwid() is not currently used anywhere anyway,
I'll take Olof's advice and just drop it. We can always add it back if
it becomes necessary.

> This of course creates the problem of associating the GPIOs to a
> driver and how it should look that up, which I guess ACPI can do,
> isn't that what acpi_find_gpio() is for?

The only one that remains is pch_gbe and we have a PCI Subsystem ID
there that we can use. Once we determine it is a MinnowBoard, we can
look the GPIO up by chip and offset. If this changes in future board
revs, we'll need minnow_hwid(), or we'll have to figure something out
either in the PCI config space or via ACPI as you suggested.

Thanks Linus, appreciate all the feedback.

-- 
Darren Hart
Intel Open Source Technology Center
Yocto Project - Technical Lead - Linux Kernel

--
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/