Hi Mugunthan,
On 3 May 2016 at 09:36, Mugunthan V N <mugunthan...@ti.com> wrote:
>
> On Monday 02 May 2016 12:24 AM, Simon Glass wrote:
> > Hi Mugunthan,
> >
> > On 28 April 2016 at 04:06, Mugunthan V N <mugunthan...@ti.com> wrote:
> >> Provide an api to check whether the given device or machine is
> >> compatible with the given compat string which helps in making
> >> decisions in drivers based on device or machine compatible.
> >>
> >> Idea taken from Linux.
> >>
> >> Signed-off-by: Mugunthan V N <mugunthan...@ti.com>
> >> Reviewed-by: Joe Hershberger <joe.hershber...@ni.com>
> >> ---
> >> drivers/core/device.c | 14 ++++++++++++++
> >> include/dm/device.h | 23 +++++++++++++++++++++++
> >> 2 files changed, 37 insertions(+)
> >>
> >> diff --git a/drivers/core/device.c b/drivers/core/device.c
> >> index 1322991..8fdd193 100644
> >> --- a/drivers/core/device.c
> >> +++ b/drivers/core/device.c
> >> @@ -715,3 +715,17 @@ int device_set_name(struct udevice *dev, const char
> >> *name)
> >>
> >> return 0;
> >> }
> >> +
> >> +bool of_device_is_compatible(struct udevice *dev, const char *compat)
> >
> > This function is in device.h, so I think device_is_compatible() is a
> > better name. Where does compat come from? Is it another device, or
> > something else entirely?
>
> I have used the funtion names as is from the kernel so that porting
> kernel driver to U-boot will be easier.
>
> The compat comes from the driver which uses the API, compat is the
> device compatible string like "ti,am3517-emac"
OK, please update the function comment. Also what do you think about
adding a new of.h /of.c files to hold this function prototype? I
really want to keep the APIs clean.
>
> >
> >> +{
> >> + const void *fdt = gd->fdt_blob;
> >> +
> >> + return !fdt_node_check_compatible(fdt, dev->of_offset, compat);
> >> +}
> >> +
> >> +bool of_machine_is_compatible(const char *compat)
> >
> > This should go in fdtdec.h I think. It doesn't have anything to do
> > with devices. So fdtdec_machine_is_compatible().
>
> I have used the funtion names as is from the kernel so that porting
> kernel driver to U-boot will be easier.
Does one function really matter? In that case, it should still go in
fdtdec, you can keep the of_ prefix. Perhaps we should use that more
generally. My reason for not is that the current fdtdec API is not the
same as of_. It uses (const void *blob, int offset) instead of (struct
of_node *node), etc.
>
> >
> >> +{
> >> + const void *fdt = gd->fdt_blob;
> >> +
> >> + return !fdt_node_check_compatible(fdt, 0, compat);
> >> +}
> >> diff --git a/include/dm/device.h b/include/dm/device.h
> >> index 8970fc0..cd18e82 100644
> >> --- a/include/dm/device.h
> >> +++ b/include/dm/device.h
> >> @@ -532,6 +532,29 @@ bool device_is_last_sibling(struct udevice *dev);
> >> int device_set_name(struct udevice *dev, const char *name);
> >>
> >> /**
> >> + * of_device_is_compatible() - check if the device is compatible with the
> >> compat
> >> + *
> >> + * This allows to check whether the device is comaptible with the compat.
> >> + *
> >> + * @dev: udevice pointer for which compatible needs to be verified.
> >> + * @compat: Compatible string which needs to verified in the given
> >> + * device
> >> + * @return true if OK, false if the compatible is not found
> >
> > Does this mean false if @compat is not found in the device's
> > compatible string list? Can you beef up the comment a bit to be
> > specific?
>
> Yes, return true if compatible is found. Will modify like below.
>
> @return true if the compatible is found, false if the compatible is not
> found
>
> >
> >> + */
> >> +bool of_device_is_compatible(struct udevice *dev, const char *compat);
> >> +
> >> +/**
> >> + * of_machine_is_compatible() - check if the machine is compatible with
> >> + * the compat
> >> + *
> >> + * This allows to check whether the machine is comaptible with the compat.
> >
> > Again can you beef up the comment? What is machine? Where does it actually
> > look?
> >
>
> Will do.
>
> >> + *
> >> + * @compat: Compatible string which needs to verified
> >> + * @return true if OK, false if the compatible is not found
> >> + */
> >> +bool of_machine_is_compatible(const char *compat);
> >> +
> >> +/**
> >> * device_is_on_pci_bus - Test if a device is on a PCI bus
> >> *
> >> * @dev: device to test
> >> --
> >> 2.8.1.339.g3ad15fd
> >>
> >
>
> Regards
> Mugunthan V N
Regards,
Simon
_______________________________________________
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot
