linux-kernel - Re: [PATCH 01/15] drivers: phy: add generic PHY framework

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:	Tue, 23 Jul 2013 15:36:00 -0400 (EDT)
From:	Alan Stern <stern@...land.harvard.edu>
To:	Tomasz Figa <t.figa@...sung.com>
cc:	Greg KH <gregkh@...uxfoundation.org>,
	Kishon Vijay Abraham I <kishon@...com>,
	Tomasz Figa <tomasz.figa@...il.com>,
	Laurent Pinchart <laurent.pinchart@...asonboard.com>,
	<broonie@...nel.org>,
	Sylwester Nawrocki <sylvester.nawrocki@...il.com>,
	Sascha Hauer <s.hauer@...gutronix.de>,
	<kyungmin.park@...sung.com>, <balbi@...com>, <jg1.han@...sung.com>,
	<s.nawrocki@...sung.com>, <kgene.kim@...sung.com>,
	<grant.likely@...aro.org>, <tony@...mide.com>, <arnd@...db.de>,
	<swarren@...dia.com>, <devicetree@...r.kernel.org>,
	<linux-doc@...r.kernel.org>, <linux-kernel@...r.kernel.org>,
	<linux-arm-kernel@...ts.infradead.org>,
	<linux-samsung-soc@...r.kernel.org>, <linux-omap@...r.kernel.org>,
	<linux-usb@...r.kernel.org>, <linux-media@...r.kernel.org>,
	<linux-fbdev@...r.kernel.org>, <akpm@...ux-foundation.org>,
	<balajitk@...com>, <george.cherian@...com>, <nsekhar@...com>,
	<olof@...om.net>, Stephen Warren <swarren@...dotorg.org>,
	<b.zolnierkie@...sung.com>,
	Daniel Lezcano <daniel.lezcano@...aro.org>
Subject: Re: [PATCH 01/15] drivers: phy: add generic PHY framework

On Tue, 23 Jul 2013, Tomasz Figa wrote:

> IMHO it would be better if you provided some code example, but let's try to 
> check if I understood you correctly.
> 
> 8><------------------------------------------------------------------------
> 
> [Board file]
> 
> static struct phy my_phy;
> 
> static struct platform_device phy_pdev = {
> 	/* ... */
> 	.platform_data = &my_phy;
> 	/* ... */
> };
> 
> static struct platform_device phy_pdev = {

This should be controller_pdev, not phy_pdev, yes?

> 	/* ... */
> 	.platform_data = &my_phy;
> 	/* ... */
> };
> 
> [Provider driver]
> 
> struct phy *phy = pdev->dev.platform_data;
> 
> ret = phy_create(phy);
> 
> [Consumer driver]
> 
> struct phy *phy = pdev->dev.platform_data;
> 
> ret = phy_get(&pdev->dev, phy);

Or even just phy_get(&pdev->dev), because phy_get() could be smart 
enough to to set phy = dev->platform_data.

> ------------------------------------------------------------------------><8
> 
> Is this what you mean?

That's what I was going to suggest too.  The struct phy is defined in
the board file, which already knows about all the PHYs that exist in
the system.  (Or perhaps it is allocated dynamically, so that when many
board files are present in the same kernel, only the entries listed in
the board file for the current system get created.)  Then the
structure's address is stored in the platform data and made available
to both the provider and the consumer.

Even though the struct phy is defined (or allocated) in the board file,
its contents don't get filled in until the PHY driver provides the
details.

> It's technically correct, but quality of this solution isn't really nice, 
> because it's a layering violation (at least if I understood what you mean). 
> This is because you need to have full definition of struct phy in board file 
> and a structure that is used as private data in PHY core comes from 
> platform code.

You don't have to have a full definition in the board file.  Just a 
partial definition -- most of the contents can be filled in later, when 
the PHY driver is ready to store the private data.

It's not a layering violation for one region of the kernel to store 
private data in a structure defined by another part of the kernel.  
This happens all the time (e.g., dev_set_drvdata).

Alan Stern

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