Hi Greg, > On May 13, 2015, at 18:36 , Greg Kroah-Hartman <gregkh@xxxxxxxxxxxxxxxxxxx> wrote: > > On Wed, May 13, 2015 at 03:10:25PM +0300, Pantelis Antoniou wrote: >> Hi Greg, >> >>> On May 13, 2015, at 14:55 , Greg Kroah-Hartman <gregkh@xxxxxxxxxxxxxxxxxxx> wrote: >>> >>> On Wed, May 13, 2015 at 10:59:41AM +0300, Pantelis Antoniou wrote: >>>> A cape loader based on DT overlays and DT objects. >>>> >>>> This is the beaglebone cape manager which allows capes to be automatically >>>> probed and instantiated via means of a device tree overlay deduced from >>>> the part-number and version contained on the cape's EEPROM. >>>> >>>> The reference manual contains information about the specification >>>> and the contents of the EEPROM. >>>> >>>> http://beagleboard.org/static/beaglebone/latest/Docs/Hardware/BONE_SRM.pdf >>>> >>>> Documentation about the workings of the cape manager is located >>>> in Documentation/misc-devices/bone_capemgr.txt >>>> >>>> This driver is using the EEPROM framework interface to retrieve >>>> the data stored on the baseboard and cape EEPROMs. >>>> >>>> Signed-off-by: Pantelis Antoniou <pantelis.antoniou@xxxxxxxxxxxx> >>>> --- >>>> drivers/misc/Kconfig | 10 + >>>> drivers/misc/Makefile | 1 + >>>> drivers/misc/bone_capemgr.c | 1926 +++++++++++++++++++++++++++++++++++++++++++ >>>> 3 files changed, 1937 insertions(+) >>>> create mode 100644 drivers/misc/bone_capemgr.c >>>> >>>> diff --git a/drivers/misc/Kconfig b/drivers/misc/Kconfig >>>> index 006242c..f9e09e1 100644 >>>> --- a/drivers/misc/Kconfig >>>> +++ b/drivers/misc/Kconfig >>>> @@ -515,6 +515,16 @@ config VEXPRESS_SYSCFG >>>> bus. System Configuration interface is one of the possible means >>>> of generating transactions on this bus. >>>> >>>> +config BONE_CAPEMGR >>>> + tristate "Beaglebone cape manager" >>>> + depends on ARCH_OMAP2PLUS && OF >>>> + select EEPROM >>>> + select OF_OVERLAY >>>> + default n >>> >>> N is always the default, please remove. >>> >> >> OK >> >>>> + help >>>> + Say Y here to include support for automatic loading of >>>> + beaglebone capes. >>>> + >>> >>> What about if it's a module? >>> >>> >> >> It should work but it’s going to be weird (i.e. no-one has used it as such). >> I’ll update the blurb. > > You are saying it is a valid config option, so please test it as such, > or if it's not a valid config option, don't allow it. > It is a valid option, just no-one has a use for it. Who knows what kind of crazy setup the end-users will use. I’ll test it as a module to make sure. >>>> source "drivers/misc/c2port/Kconfig" >>>> source "drivers/misc/eeprom/Kconfig" >>>> source "drivers/misc/cb710/Kconfig" >>>> diff --git a/drivers/misc/Makefile b/drivers/misc/Makefile >>>> index 7d5c4cd..659b78b 100644 >>>> --- a/drivers/misc/Makefile >>>> +++ b/drivers/misc/Makefile >>>> @@ -56,3 +56,4 @@ obj-$(CONFIG_GENWQE) += genwqe/ >>>> obj-$(CONFIG_ECHO) += echo/ >>>> obj-$(CONFIG_VEXPRESS_SYSCFG) += vexpress-syscfg.o >>>> obj-$(CONFIG_CXL_BASE) += cxl/ >>>> +obj-$(CONFIG_BONE_CAPEMGR) += bone_capemgr.o >>>> diff --git a/drivers/misc/bone_capemgr.c b/drivers/misc/bone_capemgr.c >>>> new file mode 100644 >>>> index 0000000..423719c >>>> --- /dev/null >>>> +++ b/drivers/misc/bone_capemgr.c >>>> @@ -0,0 +1,1926 @@ >>>> +/* >>>> + * TI Beaglebone cape manager >>>> + * >>>> + * Copyright (C) 2012 Texas Instruments Inc. >>>> + * Copyright (C) 2012-2015 Konsulko Group. >>>> + * Author: Pantelis Antoniou <pantelis.antoniou@xxxxxxxxxxxx> >>>> + * >>>> + * This program is free software; you can redistribute it and/or modify >>>> + * it under the terms of the GNU General Public License as published by >>>> + * the Free Software Foundation; either version 2 of the License, or >>>> + * (at your option) any later version. >>> >>> I have to ask, do you really mean, "or any later version”? >>> >> >> Yes, this is purely a community thing. No evil vendor at play at all. > > I have no idea what that means. > > That this is purely work for the community. No-one pays for it. >>>> + * >>>> + * 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/module.h> >>>> +#include <linux/delay.h> >>>> +#include <linux/err.h> >>>> +#include <linux/interrupt.h> >>>> +#include <linux/completion.h> >>>> +#include <linux/platform_device.h> >>>> +#include <linux/clk.h> >>>> +#include <linux/io.h> >>>> +#include <linux/of.h> >>>> +#include <linux/of_device.h> >>>> +#include <linux/of_fdt.h> >>>> +#include <linux/slab.h> >>>> +#include <linux/pm_runtime.h> >>>> +#include <linux/pinctrl/consumer.h> >>>> +#include <linux/firmware.h> >>>> +#include <linux/err.h> >>>> +#include <linux/ctype.h> >>>> +#include <linux/string.h> >>>> +#include <linux/memory.h> >>>> +#include <linux/kthread.h> >>>> +#include <linux/wait.h> >>>> +#include <linux/file.h> >>>> +#include <linux/fs.h> >>>> +#include <linux/eeprom-consumer.h> >>>> + >>>> +/* disabled capes */ >>>> +static char *disable_partno; >>>> +module_param(disable_partno, charp, 0444); >>>> +MODULE_PARM_DESC(disable_partno, >>>> + "Comma delimited list of PART-NUMBER[:REV] of disabled capes"); >>>> + >>>> +/* enable capes */ >>>> +static char *enable_partno; >>>> +module_param(enable_partno, charp, 0444); >>>> +MODULE_PARM_DESC(enable_partno, >>>> + "Comma delimited list of PART-NUMBER[:REV] of enabled capes"); >>>> + >>>> +/* delay to scan on boot until rootfs appears */ >>>> +static int boot_scan_period = 1000; >>>> +module_param(boot_scan_period, int, 0444); >>>> +MODULE_PARM_DESC(boot_scan_period, >>>> + "boot scan period until rootfs firmware is available"); >>> >>> Ick, no module parameters please, can't we drop these? >>> >> >> In a nutshell, no :) >> >> These has to be way to control whether a cape is disabled (if found) and enabled (if the manufacturer in it’s infinite wisdom removed the EEPROM to save $0.01 per cape). > > Please line-wrap your responses. > >> The easiest way to achieve this is with the kernel command line. > > No it is not. You can't easily change the kernel command line for an > embedded system (or so everyone keeps telling me when I say things like > "just update your kernel command line!") So embedded people can't have > it both ways, sorry. > Depends on the embedded system. On the ones I’m working with a sane bootloader (for example u-boot) the command line is easily modified. > Can't you put these in a dt file? > Do you really want to start this argument? Didn’t we used to have great flamewars about putting linux specific things in the DT? Does enabling & disabling capes fall into ‘hardware description’? I guess we’ll have to ask the DT maintainers for that. Rob, Grant? >>> And we have a rootfs delay option already for the whole system, this >>> module shouldn't need a special one. >>> >> >> No, that won’t work. >> >> You see the problem is as follows. >> >> The capemanager is not built as a module, it is builtin. > > Not according to your Kconfig file :) > Touché >> By the time the probe method is called the rootfs has no been mounted yet. >> This is actually what we want for the cases where the rootfs resides on a cape and >> the cape’s dtbo firmware file is built-in the kernel image. >> >> This does not work when the firmware file is located in the rootfs filesystem, since >> the call to request_firmware will fail at that time. > > Then you let it happen later. Or do it async. Don't create new command > line options for when we already have the same command line option! > Err, the two options are nothing alike no? One deals is defined as: rootdelay: "Delay (in seconds) to pause before attempting to mount the root filesystem" The other is the period with which to perform period firmware requests. >> That option controls the polling delay until the system boot state indicates that the >> rootfs is available and the call will succeed. > > If this is such an issue, just sit and spin and wait for it to show up. > I can do that; that would get rid of the option too. >> There are lots of warts in our firmware loader. > > You can fix them, don't make the kernel work around the warts because > you don't want to :) > Yes, but that’s not the case here. I have in mind fixing some problems with the firmware loader, but http://www.imdb.com/title/tt0070355/quotes?item=qt1629888 > thanks, > > greg k-h Regards — Pantelis -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
