Re: [PATCH 01/15] drivers: phy: add generic PHY framework
On Sat, Jul 20, 2013 at 10:32:26PM -0400, Alan Stern wrote: > On Sat, 20 Jul 2013, Greg KH wrote: > > > > >>That should be passed using platform data. > > > > > > > >Ick, don't pass strings around, pass pointers. If you have platform > > > >data you can get to, then put the pointer there, don't use a "name". > > > > > > I don't think I understood you here :-s We wont have phy pointer > > > when we create the device for the controller no?(it'll be done in > > > board file). Probably I'm missing something. > > > > Why will you not have that pointer? You can't rely on the "name" as the > > device id will not match up, so you should be able to rely on the > > pointer being in the structure that the board sets up, right? > > > > Don't use names, especially as ids can, and will, change, that is going > > to cause big problems. Use pointers, this is C, we are supposed to be > > doing that :) > > Kishon, I think what Greg means is this: The name you are using must > be stored somewhere in a data structure constructed by the board file, > right? Or at least, associated with some data structure somehow. > Otherwise the platform code wouldn't know which PHY hardware > corresponded to a particular name. > > Greg's suggestion is that you store the address of that data structure > in the platform data instead of storing the name string. Have the > consumer pass the data structure's address when it calls phy_create, > instead of passing the name. Then you don't have to worry about two > PHYs accidentally ending up with the same name or any other similar > problems. Close, but the issue is that whatever returns from phy_create() should then be used, no need to call any "find" functions, as you can just use the pointer that phy_create() returns. Much like all other class api functions in the kernel work. thanks, greg k-h -- To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 01/15] drivers: phy: add generic PHY framework
On Sat, 20 Jul 2013, Greg KH wrote: > > >>That should be passed using platform data. > > > > > >Ick, don't pass strings around, pass pointers. If you have platform > > >data you can get to, then put the pointer there, don't use a "name". > > > > I don't think I understood you here :-s We wont have phy pointer > > when we create the device for the controller no?(it'll be done in > > board file). Probably I'm missing something. > > Why will you not have that pointer? You can't rely on the "name" as the > device id will not match up, so you should be able to rely on the > pointer being in the structure that the board sets up, right? > > Don't use names, especially as ids can, and will, change, that is going > to cause big problems. Use pointers, this is C, we are supposed to be > doing that :) Kishon, I think what Greg means is this: The name you are using must be stored somewhere in a data structure constructed by the board file, right? Or at least, associated with some data structure somehow. Otherwise the platform code wouldn't know which PHY hardware corresponded to a particular name. Greg's suggestion is that you store the address of that data structure in the platform data instead of storing the name string. Have the consumer pass the data structure's address when it calls phy_create, instead of passing the name. Then you don't have to worry about two PHYs accidentally ending up with the same name or any other similar problems. Alan Stern -- To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 01/15] drivers: phy: add generic PHY framework
On Sat, Jul 20, 2013 at 08:49:32AM +0530, Kishon Vijay Abraham I wrote: > Hi, > > On Saturday 20 July 2013 05:20 AM, Greg KH wrote: > >On Fri, Jul 19, 2013 at 12:06:01PM +0530, Kishon Vijay Abraham I wrote: > >>Hi, > >> > >>On Friday 19 July 2013 11:59 AM, Greg KH wrote: > >>>On Fri, Jul 19, 2013 at 11:25:44AM +0530, Kishon Vijay Abraham I wrote: > Hi, > > On Friday 19 July 2013 11:13 AM, Greg KH wrote: > >On Fri, Jul 19, 2013 at 11:07:10AM +0530, Kishon Vijay Abraham I wrote: > >>+ ret = dev_set_name(&phy->dev, "%s.%d", dev_name(dev), id); > > > >Your naming is odd, no "phy" anywhere in it? You rely on the sender > >to > >never send a duplicate name.id pair? Why not create your own ids > >based > >on the number of phys in the system, like almost all other classes > >and > >subsystems do? > > hmm.. some PHY drivers use the id they provide to perform some of > their > internal operation as in [1] (This is used only if a single PHY > provider > implements multiple PHYS). Probably I'll add an option like > PLATFORM_DEVID_AUTO > to give the PHY drivers an option to use auto id. > > [1] -> > http://archive.arm.linux.org.uk/lurker/message/20130628.134308.4a8f7668.ca.html > >>> > >>>No, who cares about the id? No one outside of the phy core ever > >>>should, > >>>because you pass back the only pointer that they really do care about, > >>>if they need to do anything with the device. Use that, and then you > >>>can > >> > >>hmm.. ok. > >> > >>>rip out all of the "search for a phy by a string" logic, as that's not > >> > >>Actually this is needed for non-dt boot case. In the case of dt boot, > >>we use a > >>phandle by which the controller can get a reference to the phy. But in > >>the case > >>of non-dt boot, the controller can get a reference to the phy only by > >>label. > > > >I don't understand. They registered the phy, and got back a pointer to > >it. Why can't they save it in their local structure to use it again > >later if needed? They should never have to "ask" for the device, as the > > One is a *PHY provider* driver which is a driver for some PHY device. > This will > use phy_create to create the phy. > The other is a *PHY consumer* driver which might be any controller driver > (can > be USB/SATA/PCIE). The PHY consumer will use phy_get to get a reference > to the > phy (by *phandle* in the case of dt boot and *label* in the case of > non-dt boot). > >device id might be unknown if there are multiple devices in the system. > > I agree with you on the device id part. That need not be known to the PHY > driver. > >>> > >>>How does a consumer know which "label" to use in a non-dt system if > >>>there are multiple PHYs in the system? > >> > >>That should be passed using platform data. > > > >Ick, don't pass strings around, pass pointers. If you have platform > >data you can get to, then put the pointer there, don't use a "name". > > I don't think I understood you here :-s We wont have phy pointer > when we create the device for the controller no?(it'll be done in > board file). Probably I'm missing something. Why will you not have that pointer? You can't rely on the "name" as the device id will not match up, so you should be able to rely on the pointer being in the structure that the board sets up, right? Don't use names, especially as ids can, and will, change, that is going to cause big problems. Use pointers, this is C, we are supposed to be doing that :) thanks, greg k-h -- To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH RESEND] ARM: plat-samsung: Fix switching FIFO in arch_enable_uart_fifo function
On Tue, 2 Jul 2013 23:16:33 +0400 Alexander Shiyan wrote: > When CONFIG_S3C_BOOT_UART_FORCE_FIFO symbol is set, we should > enable FIFO but actually switch command is missing in the code. > This patch adds this switching. > > Signed-off-by: Alexander Shiyan > --- > arch/arm/plat-samsung/include/plat/uncompress.h | 2 ++ > 1 file changed, 2 insertions(+) > > diff --git a/arch/arm/plat-samsung/include/plat/uncompress.h > b/arch/arm/plat-samsung/include/plat/uncompress.h > index 4afc32f..f48dc0a 100644 > --- a/arch/arm/plat-samsung/include/plat/uncompress.h > +++ b/arch/arm/plat-samsung/include/plat/uncompress.h > @@ -145,6 +145,8 @@ static inline void arch_enable_uart_fifo(void) > if (!(fifocon & S3C2410_UFCON_RESETBOTH)) > break; > } > + > + uart_wr(S3C2410_UFCON, S3C2410_UFCON_FIFOMODE); > } > } > #else > -- > 1.8.1.5 Ping. -- Alexander Shiyan -- To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 10/22] ARM: ux500: Remove '0x's from Exynos5420 DTS file
On Fri, 19 Jul 2013, Russell King - ARM Linux wrote: > On Fri, Jul 19, 2013 at 02:58:41PM +0100, Lee Jones wrote: > > Cc: Kukjin Kim > > Cc: linux-samsung-soc@vger.kernel.org > > Signed-off-by: Lee Jones > > --- > > arch/arm/boot/dts/exynos5420.dtsi | 2 +- > > One question. What have all these files got to do with ux500 ? They don't, sorry about that. Force of habit. I'll resubmit on Monday. -- Lee Jones Linaro ST-Ericsson Landing Team Lead Linaro.org │ Open source software for ARM SoCs Follow Linaro: Facebook | Twitter | Blog -- To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
