[...]
@@ -295,12 +282,77 @@ static int mxc_gpio_probe(struct udevice *dev) return 0; } +#ifdef CONFIG_OF_CONTROL +static struct gpio_regs *mxc_get_gpio_addr(struct udevice *device) +{
- fdt_addr_t addr;
- addr = fdtdec_get_addr(gd->fdt_blob, device->of_offset, "reg");
- if (addr == FDT_ADDR_T_NONE)
return NULL;- else
return (struct gpio_regs *)addr;+} +#else +static struct gpio_regs *mxc_get_gpio_addr(struct udevice *device) +{
- return NULL;
+} +#endif
In general, I'm fine with this concept, but I believe we should implement a stub for fdtdec_get_addr() function in the fdtdec.h (say just returning FDT_ADDR_T_NONE), as otherwise we might end up with multiple drivers implementing the same noop callback just to work around a poor fdtdec_*() interface.
I tried to implement a stub function in fdtdec.h like this: __weak fdt_addr_t fdtdec_get_addr_wrap(xxxx) { return FDT_ADDR_T_NONE; } And in driver code, implement non weak version as following: #ifdef CONFIG_OF_CONTROL fdt_addr_t fdtdec_get_addr_wrap(xxxx) { .......... } #endif But gcc complains about conficting types, since we have a weak implementation in header file and a strong implementation in c file. If the weak one is in fdtxx c file, no error, but i thinke this is not a good idea to put this in fdtxx c file. If we do not want DT, but only DM, DT code should not be compiled into the final image.
Right. Putting the __weak function inside fdtxx c file will not work either as it is not compiled for !CONFIG_OF_CONTROL.
I tried another way, add the following piece code in driver/core/device.c and function prototype in device.h, " #ifdef CONFIG_OF_CONTROL void *dev_reg_addr(struct udevice *dev) { fdt_addr_t addr;
addr = fdtdev_get_addr(gd->fdt_blob, dev->of_offset, "reg"); if (addr == FDT_ADDR_T_NONE) return NULL; else return (void *)addr} #else void *dev_reg_addr(struct udevice *dev) { return NULL; } #endif " I think `#ifdef` is needed here. I think this way is better that put stub function in fdtdec.h. Using this way, the driver code can just `add = dev_reg_addr(device)` to get reg address.
You will need to check "if (!add) ..." in the driver anyway...
Yes, I agree - abstracting the dev_reg_addr() function is a great idea! It will improve the situation for all drivers that will use dev_get_addr().
Also, I think that in *addition* to the above, implementing a stub for fdtdev_get_addr() in fdtdec.h will make it even better, so you will not need the ifdef in driver/core/device.c too and also improve the fdtdec interface flexibility for any other (whatever will it be) case the driver/other code will need to call fdtdev_get_addr() explicitly.
Having said the above, I must say that I'm really a fan of how Linux interfaces deals with the CONFIG_* options, especially DT related ones.
So, I think that implementing your idea in driver/core/device.c is good enough for merging. Implementing the stub in fdtdec.h can be a bonus for all of us...
Maybe the upper piece code should be put in a new file named device-util.c in directory device/core but not device.c?
Well, I think new file will not have any real improvement over the above ideas and concepts.
[...]