On Thu, May 15, 2014 at 10:49:37AM +0530, Rahul Sharma wrote: > Hi Thierry, > > On 15 May 2014 03:44, Thierry Reding <thierry.reding@xxxxxxxxx> wrote: > > On Thu, May 15, 2014 at 12:47:21AM +0530, Rahul Sharma wrote: [...] > >> +#define HDMI_PHY 0 > >> +#define DAC_PHY 1 > >> +#define ADC_PHY 2 > >> +#define PCIE_PHY 3 > >> +#define SATA_PHY 4 > > > > Perhaps these should be namespaced somehow to avoid potential conflicts > > with other PHY providers? > > How about XXX_SIMPLE_PHY? The indices are specific to the Exynos PHY device, so why not PHY_EXYNOS_SIMPLE_XXX (or any variation thereof)? > >> +#define PHY_NR 5 > > > > I'm not sure that this belongs here either. It's not a value that will > > ever appear in a DT source file. > > I want it to grow along with new additions in the phy list else > catastrophic. This will look unrelated in driver. But this is in no way growing automatically as it is. Whoever adds a new type of PHY will need to manually increment this define. Furthermore the driver will need to be updated to cope with this anyway. I think this is even a reason to have this only in the driver. Otherwise you could have a situation where somebody upgrades the binding (and this file along with it) but not update the driver with the necessary support. But the driver will still pick up the PHY_NR change and will accept the new PHY type as valid, even though it has no code to handle it. If you have this in the driver, however, then as long as the driver has not yet been updated it will reject requests for the new PHY type. And that's exactly what it should be doing since it doesn't know how to handle it. Thierry
Attachment:
pgphIHKpx9RHd.pgp
Description: PGP signature
