new ofw_search_compatible()

[Warner Losh]

I like this...

For PC Card I took a slightly different tact where I passed in the size of the element and returned a pointer to the element. Each element started with a struct that described what to match. Worked well there, but most of the time all that was looked up was a simple int. A more general solution, but maybe not worth the extra hair, unless you combine it with a probe routine that is 100% driven from tables (which the pc card client drivers were).

The only other nit in the interface is that it requires a NULL terminator. I'd prefer a count of elements, but I could go either way.

Warner

On Oct 23, 2013, at 8:39 AM, Ian Lepore wrote:

> While creating drivers that work for a variety of SoCs, I increasingly
> find myself coding sequences such as:
> 
> 	if (ofw_bus_is_compatible(dev, "fsl,imx51-fec"))
> 		sc->fectype = FECTYPE_IMX51;
> 	else if (ofw_bus_is_compatible(dev, "fsl,imx53-fec"))
> 		sc->fectype = FECTYPE_IMX53;
> 	else if (ofw_bus_is_compatible(dev, "fsl,imx6q-fec"))
> 		sc->fectype = FECTYPE_IMX6;
> 	else
> 		sc->fectype = FECTYPE_GENERIC;
> 
> That's a short list as an example, eventually that driver may support a
> dozen SoCs.  I'd like to add a helper routine that turns this into a
> table lookup, patch attached.  Any objections?  It turns sequences such
> as the above into:
> 
> static struct ofw_compat_data compat_data[] = {
> 	{"fsl,imx51-fec", FECTYPE_IMX51},
> 	{"fsl,imx53-fec", FECTYPE_IMX53},
> 	{"fsl,imx6q-fec", FECTYPE_IMX6},
> 	{NULL,		  FECTYPE_NONE},
> };
> /* ... */
> sc->fectype = ofw_bus_search_compatible(dev, compat_data)->ocd_data;
> 
> The search routine by design can't return NULL unless you pass it a NULL
> table pointer.  That lets you provide whatever "not found" value in the
> table-end sentry that works best for the way your code is structured.
> 
> -- Ian
> 
> Index: sys/dev/ofw/ofw_bus_subr.h
> ===================================================================
> --- sys/dev/ofw/ofw_bus_subr.h	(revision 256962)
> +++ sys/dev/ofw/ofw_bus_subr.h	(working copy)
> @@ -47,6 +47,11 @@ struct ofw_bus_iinfo {
> 	pcell_t			opi_addrc;
> };
> 
> +struct ofw_compat_data {
> +	const char	*ocd_str;
> +	uintptr_t	 ocd_data;
> +};
> +
> /* Generic implementation of ofw_bus_if.m methods and helper routines */
> int	ofw_bus_gen_setup_devinfo(struct ofw_bus_devinfo *, phandle_t);
> void	ofw_bus_gen_destroy_devinfo(struct ofw_bus_devinfo *);
> @@ -74,6 +79,10 @@ void	ofw_bus_find_iparent(phandle_t);
> int ofw_bus_is_compatible(device_t, const char *);
> int ofw_bus_is_compatible_strict(device_t, const char *);
> 
> +/* Helper routine to search a list of compat props. */
> +const struct ofw_compat_data *
> +    ofw_bus_search_compatible(device_t, const struct ofw_compat_data *);
> +
> /* Helper routine for checking existence of a prop */
> int ofw_bus_has_prop(device_t, const char *);
> Index: sys/dev/ofw/ofw_bus_subr.c
> ===================================================================
> --- sys/dev/ofw/ofw_bus_subr.c	(revision 256962)
> +++ sys/dev/ofw/ofw_bus_subr.c	(working copy)
> @@ -197,6 +197,21 @@ ofw_bus_is_compatible_strict(device_t dev, const c
> 	return (0);
> }
> 
> +const struct ofw_compat_data *
> +ofw_bus_search_compatible(device_t dev, const struct ofw_compat_data *compat)
> +{
> +
> +	if (compat == NULL)
> +		return NULL;
> +
> +	for (; compat->ocd_str != NULL; ++compat) {
> +		if (ofw_bus_is_compatible(dev, compat->ocd_str))
> +			break;
> +	}
> +
> +	return (compat);
> +}
> +
> int
> ofw_bus_has_prop(device_t dev, const char *propname)
> {
> 
> _______________________________________________
> freebsd-embedded at freebsd.org mailing list
> http://lists.freebsd.org/mailman/listinfo/freebsd-embedded
> To unsubscribe, send any mail to "freebsd-embedded-unsubscribe at freebsd.org"