On Tuesday 23 of July 2013 10:37:11 Greg KH wrote: > On Tue, Jul 23, 2013 at 06:50:29PM +0200, Tomasz Figa wrote: > > > Ick, no. Why can't you just pass the pointer to the phy itself? If > > > you > > > had a "priv" pointer to search from, then you could have just passed > > > the > > > original phy pointer in the first place, right? > > > > IMHO it would be better if you provided some code example, but let's > > try to check if I understood you correctly. > > It's not my code that I want to have added, so I don't have to write > examples, I just get to complain about the existing stuff :)
Still, I think that some small code snippets illustrating the idea are really helpful. > > 8><-------------------------------------------------------------------- > > ---- > > > > [Board file] > > > > static struct phy my_phy; > > > > static struct platform_device phy_pdev = { > > > > /* ... */ > > .platform_data = &my_phy; > > /* ... */ > > > > }; > > > > static struct platform_device phy_pdev = { > > > > /* ... */ > > .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); > > > > ----------------------------------------------------------------------- > > -><8 > > > > Is this what you mean? > > No. Well, kind of. What's wrong with using the platform data structure > unique to the board to have the pointer? > > For example (just randomly picking one), the ata-pxa driver would change > include/linux/platform_data/ata-pxa.h to have a phy pointer in it: > > struct phy; > > struct pata_pxa_pdata { > /* PXA DMA DREQ<0:2> pin */ > uint32_t dma_dreq; > /* Register shift */ > uint32_t reg_shift; > /* IRQ flags */ > uint32_t irq_flags; > /* PHY */ > struct phy *phy; > }; > > Then, when you create the platform, set the phy* pointer with a call to > phy_create(). Then you can use that pointer wherever that plaform data > is available (i.e. whereever platform_data is at). Hmm? So, do you suggest to call phy_create() from board file? What phy_ops struct and other hardware parameters would it take? > > > The issue is that a string "name" is not going to scale at all, as it > > > requires hard-coded information that will change over time (as the > > > existing clock interface is already showing.) > > > > I fully agree that a simple, single string will not scale even in some, > > not so uncommon cases, but there is already a lot of existing lookup > > solutions over the kernel and so there is no point in introducing > > another one. > I'm trying to get _rid_ of lookup "solutions" and just use a real > pointer, as you should. I'll go tackle those other ones after this one > is taken care of, to show how the others should be handled as well. There was a reason for introducing lookup solutions. The reason was that in board file there is no way to get a pointer to something that is going to be created much later in time. We don't do time travel ;-). > > > Please just pass the real "phy" pointer around, that's what it is > > > there > > > for. Your "board binding" logic/code should be able to handle this, > > > as > > > it somehow was going to do the same thing with a "name". > > > > 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. > > No, just a pointer, you don't need the "full" structure until you get to > some .c code that actually manipulates the phy itself, for all other > places, you are just dealing with a pointer and a structure you never > reference. > > Does that make more sense? Well, to the point that I think I now understood your suggestion. Unfortunately the suggestion alone isn't really something that can be done, considering how driver core and generic frameworks work. Best regards, Tomasz -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html