On Mon, Nov 04, 2013 at 09:43:22PM +0100, Arnd Bergmann wrote:
> On Monday 04 November 2013 09:37:07 Stephen Warren wrote:
> > > The basic idea is to extend 'devres' to automatically register
> > > all the resources (registers, irq, dma, gpio, pinctrl, clk, regulator, ...)
> > > and simple properties before the ->probe() callback is even called,
> > > based on a per-driver data structure that describes them, and that
> > > can be easily parsed by an external tool.
> >
> > I had suggested that while talking to someone at the kernel summit,
> > basically each driver supplies functions like:
> >
> > 1) ACPI -> platform data or resources converter
> > 2) DT -> platform or resources data converter
> > 3) anything else -> platform or resources data converter
> > 4) probe()
>
> FWIW, here is a very early draft of the interfaces I have in mind.
> At the moment the implementation is DT focused, but that should
> be extensible to ACPI if necessary.
>
> At the end, you can see how a probe function could end up looking.
> I'm sure this is full of bugs at the moment, incomplete and needs
> to be moved into actual header and C files, but it should be enough
> to get across where I'm getting with this, and to see if anyone
> thinks it's a really bad idea or won't actually work.
I know that this is completely unconstructive, but the below gives me
the creeps.
Thierry
>
> Arnd
>
> #if 0
> /* allocate drvdata */
> DEVM_ALLOC,
>
> /* request hardware properties */
> DEVM_IRQ,
> DEVM_IOMAP,
> DEVM_GPIO,
> DEVM_DMA_SLAVE,
> DEVM_PINCTRL,
> DEVM_REGULATOR,
> DEVM_CLK,
> DEVM_PWM,
> DEVM_USBPHY,
>
> /* auxiliary information */
> DEVM_PROP_BOOL
> DEVM_PROP_U32
> DEVM_PROP_STRING
> #endif
>
> #error don't bother compiling this file, it's only proof-of-concept
>
> struct device;
>
> struct devm_probe {
> int (*initfunc)(struct device *dev, const struct devm_probe *probe);
> ptrdiff_t offset;
> unsigned int index;
> const char *name;
> void *arg;
> unsigned int flags;
> };
>
> /* like offsetof(), but ensures that MEMBER is of type MEMBERTYPE */
> #define offsetof_t(TYPE, MEMBER, MEMBERTYPE) \
> ((typeof(MEMBER)*)0 - typeof(MEMBERTYPE*)0 + offsetof((TYPE), (MEMBER)))
>
> /* cast 'ptr' to void* and cause a build warning if it wasn't of 'type' before */
> #define typecheck_ptr(ptr, type) \
> (void *)(ptr - (type)NULL + (type)NULL)
>
> /*
> * This is the main entry point for drivers:
> *
> * Given an zero-terminated array of 'struct devm_probe' entries,
> * this calls all initfunc pointers in the given order. Each initfunc
> * may manipulate a field in the driver_data, as pointed to by
> * the 'offset' member.
> */
> int devm_probe(struct device *dev, const struct devm_probe *probe)
> {
> int ret;
>
> for (ret = 0; !ret && probe->initfunc, probe++)
> ret = probe->initfunc(dev, probe);
>
> return ret;
> }
> EXPORT_SYMBOL_GPL(devm_probe);
>
> /*
> * this must be the called before any of the others, or not at
> * all, if dev_set_drvdata() has already been called.
> */
> static void devm_probe_alloc_release(struct device *dev, void *res)
> {
> dev_set_drvdata(dev, NULL);
> }
> int devm_probe_alloc(struct device *dev, const struct devm_probe *probe)
> {
> void *dr;
>
> if (dev_get_drvdata)
> return -EBUSY;
>
> dr = alloc_dr(devm_probe_alloc_release, probe->offset, GFP_KERNEL);
> if (unlikely(!dr))
> return -ENOMEM;
>
> dev_set_drvdata(dev, dr);
> set_node_dbginfo(&dr->node, "devm_probe_alloc", size);
> devres_add(dev, dr->data);
> }
> EXPORT_SYMBOL_GPL(devm_probe_alloc);
>
>
> #define DEVM_ALLOC(_struct) \
> { .initfunc = devm_probe_alloc, .offset = sizeof(struct _struct), }
>
> int devm_probe_irq(struct device *dev, const struct devm_probe *probe)
> {
> int ret;
> int irq;
> int *irqp;
> int index;
> const char *name;
>
> /* come up with a reasonable string for the irq name,
> maybe create devm_kasprintf() to allow arbitrary length? */
> name = devm_kmalloc(dev, 32, GFP_KERNEL);
> if (!name)
> return -ENOMEM;
> if (probe->name)
> snprintf(name, 32 - 1, "%s:%s", dev_name(dev), probe->name);
> else
> snprintf(name, 32 - 1, "%s:%n", dev_name(dev), probe->index);
>
> /* find IRQ number from resource if platform device */
> irq = 0;
> if (dev->bus_type == &platform_bus) {
> struct platform_device *pdev = to_platform_device(dev);
>
> if (probe->name)
> irq = platform_get_irq_byname(pdev, probe->name);
> else
> irq = platform_get_irq(pdev, probe->index);
> }
>
> /* try getting irq number from device tree if that failed */
> if (!irq && dev->of_node) {
> if (probe->name)
> index = of_property_match_string(dev->of_node,
> "interrupt-names",
> probe->name);
> else
> index = probe->index;
>
> irq = irq_of_parse_and_map(dev->of_node, index);
> }
>
> /* copy irq number to driver data */
> irqp = dev_get_drvdata(dev) + probe->offset;
> *irqp = irq;
>
> return devm_request_irq(dev, irq, probe->arg, probe->flags, name, probe->dev);
> }
> EXPORT_SYMBOL_GPL(devm_probe_irq);
>
> #define DEVM_IRQ(_struct, _member, _index, _handler, _flags) { \
> .initfunc = devm_probe_irq, \
> .offset = offsetof_t(struct _struct, _member, int), \
> .index = _index, \
> .arg = typecheck_ptr((_handler), irq_handler_t), \
> .flags = _flags, \
> }
>
> #define DEVM_IRQ_NAMED(_struct, _member, _name, _handler, _flags) { \
> .initfunc = devm_probe_irq, \
> .offset = offsetof_t(struct _struct, _member, int), \
> .name = _name, \
> .arg = typecheck_ptr((_handler), irq_handler_t), \
> .flags = _flags, \
> }
>
>
> #define DEVM_IOMAP_NOREQUEST 1
>
> int devm_probe_iomap(struct device *dev, const struct devm_probe *probe)
> {
> struct resource res, *resp;
> void __iomem *vaddr;
> void * __iomem *vaddrp;
> int ret;
>
> /* find mem resource from platform device */
> if (dev->bus_type == &platform_bus) {
> struct platform_device *pdev = to_platform_device(dev);
>
> if (probe->name)
> resp = platform_get_resource_byname(pdev,
> IORESOURCE_MEM, probe->name);
> else
> resp = platform_get_resource(pdev,
> IORESOURCE_MEM, probe->index);
> }
>
> /* try getting resource from device tree if that failed */
> if (!resp && dev->of_node) {
> if (probe->name)
> index = of_property_match_string(dev->of_node,
> "reg-names",
> probe->name);
> else
> index = probe->index;
>
> ret = of_address_to_resource(dev->of_node, index, &res);
> if (ret)
> return ret;
> resp = &res;
> }
>
>
> if (probe->flags & DEVM_IOMAP_NOREQUEST) {
> vaddr = devm_ioremap(dev, resp->start, resource_size(resp));
> if (!vaddr)
> return -EINVAL;
> } else {
> vaddr = devm_ioremap_resource(dev, resp);
> if (IS_ERR(vaddr))
> return PTR_ERR(vaddr);
> }
>
> vaddrp = dev_get_drvdata(dev) + probe->offset;
> *vaddrp = vaddr;
>
> return 0;
> }
> EXPORT_SYMBOL_GPL(devm_probe_iomap);
>
> #define DEVM_IOMAP(_struct, _member, _index, _flags) { \
> .initfunc = devm_probe_iomap, \
> .offset = offsetof_t(struct _struct, _member, void __iomem *), \
> .index = _index, \
> .flags = _flags, \
> }
>
> #define DEVM_IOMAP_NAMED(_struct, _member, _name, _flags) { \
> .initfunc = devm_probe_iomap, \
> .offset = offsetof_t(struct _struct, _member, void __iomem *), \
> .name = _name, \
> .flags = _flags, \
> }
>
> int devm_probe_gpio(struct device *dev, const struct devm_probe *probe)
> {
> struct gpio_desc *desc, **descp;
>
> desc = devm_gpiod_get_index(dev, probe->name, probe->index);
> if (IS_ERR(desc)) {
> /* FIXME: this looks wrong */
> desc = of_get_named_gpiod_flags(dev->of_node, probe->name,
> probe->index, NULL);
> if (IS_ERR(desc))
> return PTR_ERR(desc);
> devm_gpio_request(dev, desc_to_gpio(desc), probe->name);
> }
>
> descp = dev_get_drvdata(dev) + probe->offset;
> *descp = desc;
>
> return 0;
> }
>
> #define DEVM_GPIO(_struct, _member, _index) { \
> .initfunc = devm_probe_iomap, \
> .offset = offsetof_t(struct _struct, _member, struct gpio_desc*),\
> .name = "gpios", \
> .index = _index, \
> }
>
> #define DEVM_GPIO_NAMED(_struct, _member, _name) { \
> .initfunc = devm_probe_iomap, \
> .offset = offsetof_t(struct _struct, _member, struct gpio_desc*),\
> .name = _name, \
> }
>
> static void devm_probe_dma_release(struct device *dev, void *chanp)
> {
> dma_release_channel(*(struct dma_chan**)chanp);
> }
>
> int devm_probe_dma(struct device *dev, const struct devm_probe *probe)
> {
> struct dma_chan *chan, **chanp;
>
> /* there is no devm_dma_request_channel yet, so build our own */
> chanp = devres_alloc(devm_probe_dma_release, sizeof(chanp), GFP_KERNEL);
> if (!chanp)
> return -ENOMEM;
>
> chan = dma_request_slave_channel(dev, probe->name);
> if (!chan) {
> devres_free(chanp);
> return -EINVAL;
> }
>
> *chanp = chan;
> devres_add(dev, chanp);
>
> /* add pointer to the private data */
> chanp = dev_get_drvdata(dev) + probe->offset;
> *chanp = chan;
>
> return 0;
> }
>
> #define DEVM_DMA_SLAVE(_struct, _member, _name) \
> .offset = offsetof_t(struct _struct, _member, struct dma_chan*),\
> .name = _name, \
> }
>
> /*
> * simple properties: bool, u32, string
> * no actual need for managed interfaces, just a way to abstract the
> * access to DT or other information source
> */
> int devm_probe_prop_bool(struct device *dev, struct devm_probe *probe)
> {
> bool *val;
>
> val = dev_get_drvdata(dev) + probe->offset;
> *val = of_property_read_bool(dev->of_node, probe->name);
>
> return 0;
> }
> EXPORT_SYMBOL_GPL(devm_probe_prop_bool);
>
> #define DEVM_PROP_BOOL(_struct, _member, _name) \
> .offset = offsetof_t(struct _struct, _member, bool), \
> .name = _name, \
> }
>
> int devm_probe_prop_u32(struct device *dev, struct devm_probe *probe)
> {
> u32 *val;
> int ret;
>
> val = dev_get_drvdata(dev) + probe->offset;
> ret = of_property_read_u32(dev->of_node, probe->name, val);
>
> return ret;
> }
> EXPORT_SYMBOL_GPL(devm_probe_prop_bool);
>
> #define DEVM_PROP_U32(_struct, _member, _name) \
> .offset = offsetof_t(struct _struct, _member, u32), \
> .name = _name, \
> }
>
> int devm_probe_prop_string(struct device *dev, struct devm_probe *probe)
> {
> const char **val;
> int ret;
>
> val = dev_get_drvdata(dev) + probe->offset;
> ret = of_property_read_string(dev->of_node, probe->name, val);
>
> return ret;
> }
> EXPORT_SYMBOL_GPL(devm_probe_prop_bool);
>
> #define DEVM_PROP_STRING(_struct, _member, _name) \
> .offset = offsetof_t(struct _struct, _member, const char *), \
> .name = _name, \
> }
>
> /* example driver */
> struct foo_priv {
> spinlock_t lock;
> void __iomem *regs;
> int irq;
> struct gpio_desc *gpio;
> struct dma_chan *rxdma;
> struct dma_chan *txdma;
> bool oldstyle_dma;
> };
>
> static irqreturn_t foo_irq_handler(int irq, void *dev);
>
> /*
> * this lists all properties we access from the driver. The list
> * is interpreted by devm_probe() and can be programmatically
> * verified to match the binding.
> */
> static const struct devm_probe foo_probe_list[] = {
> DEVM_ALLOC(foo_priv),
> DEVM_IOMAP(foo_priv, regs, 0, 0),
> DEVM_PROP_BOOL(foo_priv, oldstyle_dma, "foo,oldstyle-dma"),
> DEVM_DMA_SLAVE(foo_priv, rxdma, "rx");
> DEVM_DMA_SLAVE(foo_priv, txdma, "tx");
> DEVM_GPIO(foo_priv, gpio, 0);
> DEVM_IRQ_NAMED(foo_priv, irq, foo_irq_handler, "fifo", IRQF_SHARED),
> {},
> };
>
> static int foo_probe(struct platform_device *dev)
> {
> int ret;
>
> ret = devm_probe(dev->dev, foo_probe_list);
> if (ret)
> return ret;
>
> return bar_subsystem_register(&foo_bar_ops, dev);
> }
>
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel@xxxxxxxxxxxxxxxxxxx
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
Attachment:
pgp9Z3910dltu.pgp
Description: PGP signature
