linux-kernel - Re: [RFC PATCH 3/9] Driver core: Unified device properties interface for platform firmware

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <2012139.h82pgQUHg7@vostro.rjw.lan>
Date:	Mon, 18 Aug 2014 06:46:42 +0200
From:	"Rafael J. Wysocki" <rjw@...ysocki.net>
To:	Grant Likely <grant.likely@...retlab.ca>
Cc:	Mika Westerberg <mika.westerberg@...ux.intel.com>,
	Darren Hart <dvhart@...ux.intel.com>,
	"Rafael J. Wysocki" <rafael@...nel.org>,
	Aaron Lu <aaron.lu@...el.com>,
	Max Eliaser <max.eliaser@...el.com>,
	linux-acpi@...r.kernel.org, devicetree@...r.kernel.org,
	linux-kernel@...r.kernel.org
Subject: Re: [RFC PATCH 3/9] Driver core: Unified device properties interface for platform firmware

On Sunday, August 17, 2014 01:49:13 PM Grant Likely wrote:
> 
> Hi Mika and Rafael,
> 
> Comments below...

[cut]

> > +enum dev_prop_type {
> > +	DEV_PROP_U8,
> > +	DEV_PROP_U16,
> > +	DEV_PROP_U32,
> > +	DEV_PROP_U64,
> > +	DEV_PROP_STRING,
> > +	DEV_PROP_MAX,
> > +};
> > +
> > +struct dev_prop_ops {
> > +	int (*get)(struct device *dev, const char *propname, void **valptr);
> > +	int (*read)(struct device *dev, const char *propname,
> > +		    enum dev_prop_type proptype, void *val);
> > +	int (*read_array)(struct device *dev, const char *propname,
> > +			  enum dev_prop_type proptype, void *val, size_t nval);
> 
> The associated DT functions that implement property reads
> (of_property_read_*) were created in part to provide some type safety
> when reading properties. This proposed API throws that away by accepting
> a void* for the data field, which I don't want to do. This API either
> needs to have a separate accessor for each data type, or it needs some
> other mechanism (accessor macros?) to ensure the right type is passed
> in.

The intention is to add static inline functions like:

int device_property_read_u64(struct device *dev, const char *propname, u64 *val)
{
	return device_property_read(dev, propname, DEV_PROP_U64, val);
}

and so on for the other property types.  They just have not been implemented in
this version of the patch.

> 
> > +	int (*child_count)(struct device *dev);
> > +};
> > +
> > +#ifdef CONFIG_ACPI
> > +extern struct dev_prop_ops acpi_property_ops;
> > +#endif
> 
> Rendered moot by my comment about eliminating the ops structure, but the
> above shouldn't appear here. acpi_property_ops shouldn't ever be visible
> outside ACPI core code, so it shouldn't be in this header.

It doesn't look like this has to be present here.  At least this particular
patch should compile just fine after removing the 3 lines above.

That seems to be a leftover from one of the previous versions of it.

Rafael

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/