Message ID | 1481069677-53660-6-git-send-email-christopher.lee.bostic@gmail.com (mailing list archive) |
---|---|
State | New, archived |
Headers | show |
On Tue, Dec 06, 2016 at 06:14:26PM -0600, Chris Bostic wrote: > From: Jeremy Kerr <jk@ozlabs.org> > > For debugging, add a fake master driver, that only supports reads, > returning a fixed set of data. > +config FSI_MASTER_FAKE > + tristate "Fake FSI master" > + depends on FSI > + ---help--- > + This option enables a fake FSI master driver for debugging. > +endif > +static const struct of_device_id fsi_master_fake_match[] = { > + { .compatible = "ibm,fsi-master-fake" }, > + { }, > +}; NAK. DT should be treated as an ABI, and should describe the HW explicitly. This makes no sense. This is also missing a binding document. Have your module take a module parameter allowing you to bind it to arbitrary devices, or do something like what PCI does where you can bind/unbind arbitrary drivers to devices using sysfs. > + > +static struct platform_driver fsi_master_fake_driver = { > + .driver = { > + .name = "fsi-master-fake", > + .of_match_table = fsi_master_fake_match, > + }, > + .probe = fsi_master_fake_probe, > +}; > + > +static int __init fsi_master_fake_init(void) > +{ > + struct device_node *np; > + > + platform_driver_register(&fsi_master_fake_driver); > + > + for_each_compatible_node(np, NULL, "ibm,fsi-master-fake") > + of_platform_device_create(np, NULL, NULL); As a general note, please use for_each_matching_node in situations like this. That way you can reuse your existing of_device_id table, and not reproduce the string. That said, this is not necessary. The platform driver has an of_match_table, so presumes the parent bus registers children, and hence they should already have platform devices. Thanks, Mark.
Hi Mark & Chris, > On Tue, Dec 06, 2016 at 06:14:26PM -0600, Chris Bostic wrote: >> From: Jeremy Kerr <jk@ozlabs.org> >> >> For debugging, add a fake master driver, that only supports reads, >> returning a fixed set of data. > >> +config FSI_MASTER_FAKE >> + tristate "Fake FSI master" >> + depends on FSI >> + ---help--- >> + This option enables a fake FSI master driver for debugging. >> +endif > >> +static const struct of_device_id fsi_master_fake_match[] = { >> + { .compatible = "ibm,fsi-master-fake" }, >> + { }, >> +}; > > NAK. > > DT should be treated as an ABI, and should describe the HW explicitly. > This makes no sense. This is also missing a binding document. > > Have your module take a module parameter allowing you to bind it to > arbitrary devices, or do something like what PCI does where you can > bind/unbind arbitrary drivers to devices using sysfs. This driver is purely for testing the FSI engine scan code; we could probably just drop this patch since I suspect that it's no longer useful (now that we have an actual master driver). If we do want to keep it though, I'd say we remove the device tree dependency; all this is doing at the moment is triggering the ->probe, and there are better ways to do that. Cheers, Jeremy
diff --git a/drivers/fsi/Kconfig b/drivers/fsi/Kconfig index 04c1a0e..f065dbe 100644 --- a/drivers/fsi/Kconfig +++ b/drivers/fsi/Kconfig @@ -9,4 +9,14 @@ config FSI ---help--- FSI - the FRU Support Interface - is a simple bus for low-level access to POWER-based hardware. + +if FSI + +config FSI_MASTER_FAKE + tristate "Fake FSI master" + depends on FSI + ---help--- + This option enables a fake FSI master driver for debugging. +endif + endmenu diff --git a/drivers/fsi/Makefile b/drivers/fsi/Makefile index db0e5e7..847c00c 100644 --- a/drivers/fsi/Makefile +++ b/drivers/fsi/Makefile @@ -1,2 +1,3 @@ obj-$(CONFIG_FSI) += fsi-core.o +obj-$(CONFIG_FSI_MASTER_FAKE) += fsi-master-fake.o diff --git a/drivers/fsi/fsi-master-fake.c b/drivers/fsi/fsi-master-fake.c new file mode 100644 index 0000000..b42fe5b --- /dev/null +++ b/drivers/fsi/fsi-master-fake.c @@ -0,0 +1,95 @@ +/* + * Fake FSI master driver for FSI development + * + * Copyright (C) IBM Corporation 2016 + * + * This program is free software; you can redistribute it and/or modify + * it under the terms of the GNU General Public License version 2 as + * published by the Free Software Foundation. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + */ + +#include <linux/platform_device.h> +#include <linux/module.h> +#include <linux/of.h> +#include <linux/of_platform.h> + +#include "fsi-master.h" + +static const uint8_t data[] = { + 0xc0, 0x02, 0x08, 0x03, /* chip id */ + 0x80, 0x01, 0x11, 0x00, /* peek */ + 0x80, 0x01, 0x20, 0x3e, /* slave */ + 0x00, 0x01, 0x10, 0xa5, /* i2c */ +}; + + +static int fsi_master_fake_read(struct fsi_master *_master, int link, + uint8_t slave, uint32_t addr, void *val, size_t size) +{ + if (link != 0) + return -ENODEV; + + if (addr + size > sizeof(data)) + memset(val, 0, size); + else + memcpy(val, data + addr, size); + + return 0; +} + +static int fsi_master_fake_write(struct fsi_master *_master, int link, + uint8_t slave, uint32_t addr, const void *val, size_t size) +{ + if (link != 0) + return -ENODEV; + + return -EACCES; +} + +static int fsi_master_fake_probe(struct platform_device *pdev) +{ + struct fsi_master *master; + + master = devm_kzalloc(&pdev->dev, sizeof(*master), GFP_KERNEL); + if (!master) + return -ENOMEM; + + master->dev = &pdev->dev; + master->n_links = 1; + master->read = fsi_master_fake_read; + master->write = fsi_master_fake_write; + + return fsi_master_register(master); +} + +static const struct of_device_id fsi_master_fake_match[] = { + { .compatible = "ibm,fsi-master-fake" }, + { }, +}; + +static struct platform_driver fsi_master_fake_driver = { + .driver = { + .name = "fsi-master-fake", + .of_match_table = fsi_master_fake_match, + }, + .probe = fsi_master_fake_probe, +}; + +static int __init fsi_master_fake_init(void) +{ + struct device_node *np; + + platform_driver_register(&fsi_master_fake_driver); + + for_each_compatible_node(np, NULL, "ibm,fsi-master-fake") + of_platform_device_create(np, NULL, NULL); + + return 0; +} + +module_init(fsi_master_fake_init);