* [PATCH v7 11/12] hwmon: Add PECI dimmtemp driver
From: Jae Hyun Yoo @ 2018-07-30 22:45 UTC (permalink / raw)
To: linux-aspeed
In-Reply-To: <CAL_JsqK4k=c6k-BJT5fWQH_DCqGhr13fq6M_iZZehaveE-z5Hg@mail.gmail.com>
Hi Rob,
On 7/30/2018 3:06 PM, Rob Herring wrote:
> On Mon, Jul 23, 2018 at 3:49 PM Jae Hyun Yoo
> <jae.hyun.yoo@linux.intel.com> wrote:
>>
>> This commit adds PECI dimmtemp hwmon driver.
>>
>
>> +#if IS_ENABLED(CONFIG_OF)
>> +static const struct of_device_id peci_dimmtemp_of_table[] = {
>> + { .compatible = "intel,peci-dimmtemp" },
>> + { }
>> +};
>> +MODULE_DEVICE_TABLE(of, peci_dimmtemp_of_table);
>> +#endif
>
> This should be removed. Same for cpu temp driver I'm guessing.
>
Yes, you are right. This isn't needed anymore because MFD driver
loads this module. I'll remove this code from here and from cpu temp
driver. Also, I'll remove 'of_compatible' setting of 'struct mfd_cell
peci_functions' in intel-peci-client.c code.
Thanks a lot for your pointing it out!
Jae
> Rob
>
^ permalink raw reply
* [PATCH v7 08/12] mfd: intel-peci-client: Add PECI client MFD driver
From: Rob Herring @ 2018-07-30 22:10 UTC (permalink / raw)
To: linux-aspeed
In-Reply-To: <acffdfef-cb20-4208-5297-d94bac5ecb1b@linux.intel.com>
On Fri, Jul 27, 2018 at 11:36 AM Jae Hyun Yoo
<jae.hyun.yoo@linux.intel.com> wrote:
>
> Hi Lee,
>
> On 7/27/2018 1:26 AM, Lee Jones wrote:
> > On Mon, 23 Jul 2018, Jae Hyun Yoo wrote:
> >
> >> This commit adds PECI client MFD driver.
> >>
> >> Signed-off-by: Jae Hyun Yoo <jae.hyun.yoo@linux.intel.com>
> >> Cc: Lee Jones <lee.jones@linaro.org>
> >> Cc: Rob Herring <robh+dt@kernel.org>
> >> Cc: Andrew Jeffery <andrew@aj.id.au>
> >> Cc: James Feist <james.feist@linux.intel.com>
> >> Cc: Jason M Biils <jason.m.bills@linux.intel.com>
> >> Cc: Joel Stanley <joel@jms.id.au>
> >> Cc: Vernon Mauery <vernon.mauery@linux.intel.com>
> >> ---
> >> drivers/mfd/Kconfig | 14 ++
> >> drivers/mfd/Makefile | 1 +
> >> drivers/mfd/intel-peci-client.c | 182 ++++++++++++++++++++++++++
> >> include/linux/mfd/intel-peci-client.h | 81 ++++++++++++
> >> 4 files changed, 278 insertions(+)
> >> create mode 100644 drivers/mfd/intel-peci-client.c
> >> create mode 100644 include/linux/mfd/intel-peci-client.h
> >>
> >> diff --git a/drivers/mfd/Kconfig b/drivers/mfd/Kconfig
> >> index f3fa516011ec..e38b591479d4 100644
> >> --- a/drivers/mfd/Kconfig
> >> +++ b/drivers/mfd/Kconfig
> >> @@ -595,6 +595,20 @@ config MFD_INTEL_MSIC
> >> Passage) chip. This chip embeds audio, battery, GPIO, etc.
> >> devices used in Intel Medfield platforms.
> >>
> >> +config MFD_INTEL_PECI_CLIENT
> >> + bool "Intel PECI client"
> >> + depends on (PECI || COMPILE_TEST)
> >> + select MFD_CORE
> >> + help
> >> + If you say yes to this option, support will be included for the
> >> + multi-funtional Intel PECI (Platform Environment Control Interface)
> >> + client. PECI is a one-wire bus interface that provides a communication
> >> + channel from PECI clients in Intel processors and chipset components
> >> + to external monitoring or control devices.
> >> +
> >> + Additional drivers must be enabled in order to use the functionality
> >> + of the device.
> >> +
> >> config MFD_IPAQ_MICRO
> >> bool "Atmel Micro ASIC (iPAQ h3100/h3600/h3700) Support"
> >> depends on SA1100_H3100 || SA1100_H3600
> >> diff --git a/drivers/mfd/Makefile b/drivers/mfd/Makefile
> >> index 2852a6042ecf..29e2cacc58bd 100644
> >> --- a/drivers/mfd/Makefile
> >> +++ b/drivers/mfd/Makefile
> >> @@ -203,6 +203,7 @@ obj-$(CONFIG_MFD_INTEL_LPSS) += intel-lpss.o
> >> obj-$(CONFIG_MFD_INTEL_LPSS_PCI) += intel-lpss-pci.o
> >> obj-$(CONFIG_MFD_INTEL_LPSS_ACPI) += intel-lpss-acpi.o
> >> obj-$(CONFIG_MFD_INTEL_MSIC) += intel_msic.o
> >> +obj-$(CONFIG_MFD_INTEL_PECI_CLIENT) += intel-peci-client.o
> >> obj-$(CONFIG_MFD_PALMAS) += palmas.o
> >> obj-$(CONFIG_MFD_VIPERBOARD) += viperboard.o
> >> obj-$(CONFIG_MFD_RC5T583) += rc5t583.o rc5t583-irq.o
> >> diff --git a/drivers/mfd/intel-peci-client.c b/drivers/mfd/intel-peci-client.c
> >> new file mode 100644
> >> index 000000000000..d7702cf1ea50
> >> --- /dev/null
> >> +++ b/drivers/mfd/intel-peci-client.c
> >> @@ -0,0 +1,182 @@
> >> +// SPDX-License-Identifier: GPL-2.0
> >> +// Copyright (c) 2018 Intel Corporation
> >> +
> >> +#include <linux/bitfield.h>
> >> +#include <linux/mfd/core.h>
> >> +#include <linux/mfd/intel-peci-client.h>
> >> +#include <linux/module.h>
> >> +#include <linux/peci.h>
> >> +#include <linux/of_device.h>
> >> +
> >> +enum cpu_gens {
> >> + CPU_GEN_HSX = 0, /* Haswell Xeon */
> >> + CPU_GEN_BRX, /* Broadwell Xeon */
> >> + CPU_GEN_SKX, /* Skylake Xeon */
> >> +};
> >> +
> >> +static struct mfd_cell peci_functions[] = {
> >> + {
> >> + .name = "peci-cputemp",
> >> + .of_compatible = "intel,peci-cputemp",
> >> + },
> >> + {
> >> + .name = "peci-dimmtemp",
> >> + .of_compatible = "intel,peci-dimmtemp",
> >> + },
> >> +};
> >
> > The more I look at this driver, the less I think it fits into MFD.
> >
> > What's stopping you from registering these devices directly from DT?
> >
>
> Because DT doesn't allow 2 nodes at the same address so Rob suggested
> MFD for this case.
Only after I first suggested you create cpu and dimm sensors from a
single hwmon driver. Perhaps you should figure out how to fix the
problem with delayed registration. Perhaps you can register the
sensor, but just delay returning actual data until it is ready.
Rob
^ permalink raw reply
* [PATCH v7 11/12] hwmon: Add PECI dimmtemp driver
From: Rob Herring @ 2018-07-30 22:06 UTC (permalink / raw)
To: linux-aspeed
In-Reply-To: <20180723214751.1733-12-jae.hyun.yoo@linux.intel.com>
On Mon, Jul 23, 2018 at 3:49 PM Jae Hyun Yoo
<jae.hyun.yoo@linux.intel.com> wrote:
>
> This commit adds PECI dimmtemp hwmon driver.
>
> +#if IS_ENABLED(CONFIG_OF)
> +static const struct of_device_id peci_dimmtemp_of_table[] = {
> + { .compatible = "intel,peci-dimmtemp" },
> + { }
> +};
> +MODULE_DEVICE_TABLE(of, peci_dimmtemp_of_table);
> +#endif
This should be removed. Same for cpu temp driver I'm guessing.
Rob
^ permalink raw reply
* [PATCH v7 08/12] mfd: intel-peci-client: Add PECI client MFD driver
From: Jae Hyun Yoo @ 2018-07-27 17:36 UTC (permalink / raw)
To: linux-aspeed
In-Reply-To: <20180727082611.GE4628@dell>
Hi Lee,
On 7/27/2018 1:26 AM, Lee Jones wrote:
> On Mon, 23 Jul 2018, Jae Hyun Yoo wrote:
>
>> This commit adds PECI client MFD driver.
>>
>> Signed-off-by: Jae Hyun Yoo <jae.hyun.yoo@linux.intel.com>
>> Cc: Lee Jones <lee.jones@linaro.org>
>> Cc: Rob Herring <robh+dt@kernel.org>
>> Cc: Andrew Jeffery <andrew@aj.id.au>
>> Cc: James Feist <james.feist@linux.intel.com>
>> Cc: Jason M Biils <jason.m.bills@linux.intel.com>
>> Cc: Joel Stanley <joel@jms.id.au>
>> Cc: Vernon Mauery <vernon.mauery@linux.intel.com>
>> ---
>> drivers/mfd/Kconfig | 14 ++
>> drivers/mfd/Makefile | 1 +
>> drivers/mfd/intel-peci-client.c | 182 ++++++++++++++++++++++++++
>> include/linux/mfd/intel-peci-client.h | 81 ++++++++++++
>> 4 files changed, 278 insertions(+)
>> create mode 100644 drivers/mfd/intel-peci-client.c
>> create mode 100644 include/linux/mfd/intel-peci-client.h
>>
>> diff --git a/drivers/mfd/Kconfig b/drivers/mfd/Kconfig
>> index f3fa516011ec..e38b591479d4 100644
>> --- a/drivers/mfd/Kconfig
>> +++ b/drivers/mfd/Kconfig
>> @@ -595,6 +595,20 @@ config MFD_INTEL_MSIC
>> Passage) chip. This chip embeds audio, battery, GPIO, etc.
>> devices used in Intel Medfield platforms.
>>
>> +config MFD_INTEL_PECI_CLIENT
>> + bool "Intel PECI client"
>> + depends on (PECI || COMPILE_TEST)
>> + select MFD_CORE
>> + help
>> + If you say yes to this option, support will be included for the
>> + multi-funtional Intel PECI (Platform Environment Control Interface)
>> + client. PECI is a one-wire bus interface that provides a communication
>> + channel from PECI clients in Intel processors and chipset components
>> + to external monitoring or control devices.
>> +
>> + Additional drivers must be enabled in order to use the functionality
>> + of the device.
>> +
>> config MFD_IPAQ_MICRO
>> bool "Atmel Micro ASIC (iPAQ h3100/h3600/h3700) Support"
>> depends on SA1100_H3100 || SA1100_H3600
>> diff --git a/drivers/mfd/Makefile b/drivers/mfd/Makefile
>> index 2852a6042ecf..29e2cacc58bd 100644
>> --- a/drivers/mfd/Makefile
>> +++ b/drivers/mfd/Makefile
>> @@ -203,6 +203,7 @@ obj-$(CONFIG_MFD_INTEL_LPSS) += intel-lpss.o
>> obj-$(CONFIG_MFD_INTEL_LPSS_PCI) += intel-lpss-pci.o
>> obj-$(CONFIG_MFD_INTEL_LPSS_ACPI) += intel-lpss-acpi.o
>> obj-$(CONFIG_MFD_INTEL_MSIC) += intel_msic.o
>> +obj-$(CONFIG_MFD_INTEL_PECI_CLIENT) += intel-peci-client.o
>> obj-$(CONFIG_MFD_PALMAS) += palmas.o
>> obj-$(CONFIG_MFD_VIPERBOARD) += viperboard.o
>> obj-$(CONFIG_MFD_RC5T583) += rc5t583.o rc5t583-irq.o
>> diff --git a/drivers/mfd/intel-peci-client.c b/drivers/mfd/intel-peci-client.c
>> new file mode 100644
>> index 000000000000..d7702cf1ea50
>> --- /dev/null
>> +++ b/drivers/mfd/intel-peci-client.c
>> @@ -0,0 +1,182 @@
>> +// SPDX-License-Identifier: GPL-2.0
>> +// Copyright (c) 2018 Intel Corporation
>> +
>> +#include <linux/bitfield.h>
>> +#include <linux/mfd/core.h>
>> +#include <linux/mfd/intel-peci-client.h>
>> +#include <linux/module.h>
>> +#include <linux/peci.h>
>> +#include <linux/of_device.h>
>> +
>> +enum cpu_gens {
>> + CPU_GEN_HSX = 0, /* Haswell Xeon */
>> + CPU_GEN_BRX, /* Broadwell Xeon */
>> + CPU_GEN_SKX, /* Skylake Xeon */
>> +};
>> +
>> +static struct mfd_cell peci_functions[] = {
>> + {
>> + .name = "peci-cputemp",
>> + .of_compatible = "intel,peci-cputemp",
>> + },
>> + {
>> + .name = "peci-dimmtemp",
>> + .of_compatible = "intel,peci-dimmtemp",
>> + },
>> +};
>
> The more I look at this driver, the less I think it fits into MFD.
>
> What's stopping you from registering these devices directly from DT?
>
Because DT doesn't allow 2 nodes at the same address so Rob suggested
MFD for this case.
Thanks,
Jae
^ permalink raw reply
* [PATCH v7 08/12] mfd: intel-peci-client: Add PECI client MFD driver
From: Lee Jones @ 2018-07-27 8:26 UTC (permalink / raw)
To: linux-aspeed
In-Reply-To: <20180723214751.1733-9-jae.hyun.yoo@linux.intel.com>
On Mon, 23 Jul 2018, Jae Hyun Yoo wrote:
> This commit adds PECI client MFD driver.
>
> Signed-off-by: Jae Hyun Yoo <jae.hyun.yoo@linux.intel.com>
> Cc: Lee Jones <lee.jones@linaro.org>
> Cc: Rob Herring <robh+dt@kernel.org>
> Cc: Andrew Jeffery <andrew@aj.id.au>
> Cc: James Feist <james.feist@linux.intel.com>
> Cc: Jason M Biils <jason.m.bills@linux.intel.com>
> Cc: Joel Stanley <joel@jms.id.au>
> Cc: Vernon Mauery <vernon.mauery@linux.intel.com>
> ---
> drivers/mfd/Kconfig | 14 ++
> drivers/mfd/Makefile | 1 +
> drivers/mfd/intel-peci-client.c | 182 ++++++++++++++++++++++++++
> include/linux/mfd/intel-peci-client.h | 81 ++++++++++++
> 4 files changed, 278 insertions(+)
> create mode 100644 drivers/mfd/intel-peci-client.c
> create mode 100644 include/linux/mfd/intel-peci-client.h
>
> diff --git a/drivers/mfd/Kconfig b/drivers/mfd/Kconfig
> index f3fa516011ec..e38b591479d4 100644
> --- a/drivers/mfd/Kconfig
> +++ b/drivers/mfd/Kconfig
> @@ -595,6 +595,20 @@ config MFD_INTEL_MSIC
> Passage) chip. This chip embeds audio, battery, GPIO, etc.
> devices used in Intel Medfield platforms.
>
> +config MFD_INTEL_PECI_CLIENT
> + bool "Intel PECI client"
> + depends on (PECI || COMPILE_TEST)
> + select MFD_CORE
> + help
> + If you say yes to this option, support will be included for the
> + multi-funtional Intel PECI (Platform Environment Control Interface)
> + client. PECI is a one-wire bus interface that provides a communication
> + channel from PECI clients in Intel processors and chipset components
> + to external monitoring or control devices.
> +
> + Additional drivers must be enabled in order to use the functionality
> + of the device.
> +
> config MFD_IPAQ_MICRO
> bool "Atmel Micro ASIC (iPAQ h3100/h3600/h3700) Support"
> depends on SA1100_H3100 || SA1100_H3600
> diff --git a/drivers/mfd/Makefile b/drivers/mfd/Makefile
> index 2852a6042ecf..29e2cacc58bd 100644
> --- a/drivers/mfd/Makefile
> +++ b/drivers/mfd/Makefile
> @@ -203,6 +203,7 @@ obj-$(CONFIG_MFD_INTEL_LPSS) += intel-lpss.o
> obj-$(CONFIG_MFD_INTEL_LPSS_PCI) += intel-lpss-pci.o
> obj-$(CONFIG_MFD_INTEL_LPSS_ACPI) += intel-lpss-acpi.o
> obj-$(CONFIG_MFD_INTEL_MSIC) += intel_msic.o
> +obj-$(CONFIG_MFD_INTEL_PECI_CLIENT) += intel-peci-client.o
> obj-$(CONFIG_MFD_PALMAS) += palmas.o
> obj-$(CONFIG_MFD_VIPERBOARD) += viperboard.o
> obj-$(CONFIG_MFD_RC5T583) += rc5t583.o rc5t583-irq.o
> diff --git a/drivers/mfd/intel-peci-client.c b/drivers/mfd/intel-peci-client.c
> new file mode 100644
> index 000000000000..d7702cf1ea50
> --- /dev/null
> +++ b/drivers/mfd/intel-peci-client.c
> @@ -0,0 +1,182 @@
> +// SPDX-License-Identifier: GPL-2.0
> +// Copyright (c) 2018 Intel Corporation
> +
> +#include <linux/bitfield.h>
> +#include <linux/mfd/core.h>
> +#include <linux/mfd/intel-peci-client.h>
> +#include <linux/module.h>
> +#include <linux/peci.h>
> +#include <linux/of_device.h>
> +
> +enum cpu_gens {
> + CPU_GEN_HSX = 0, /* Haswell Xeon */
> + CPU_GEN_BRX, /* Broadwell Xeon */
> + CPU_GEN_SKX, /* Skylake Xeon */
> +};
> +
> +static struct mfd_cell peci_functions[] = {
> + {
> + .name = "peci-cputemp",
> + .of_compatible = "intel,peci-cputemp",
> + },
> + {
> + .name = "peci-dimmtemp",
> + .of_compatible = "intel,peci-dimmtemp",
> + },
> +};
The more I look at this driver, the less I think it fits into MFD.
What's stopping you from registering these devices directly from DT?
--
Lee Jones [???]
Linaro Services Technical Lead
Linaro.org ? Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog
^ permalink raw reply
* [GIT PULL] ARM: aspeed: dts changes for 4.19
From: Joel Stanley @ 2018-07-27 1:22 UTC (permalink / raw)
To: linux-aspeed
In-Reply-To: <20180726195127.jukxvbydddlwui4k@localhost>
On 27 July 2018 at 05:21, Olof Johansson <olof@lixom.net> wrote:
> Hi,
>
> On Thu, Jul 26, 2018 at 02:00:49PM +0930, Joel Stanley wrote:
>> Hello ARM Maintainers,
>>
>> Here are the ASPEED device tree updates for 4.19. No new machines this time
>> around, but we now have some FSI (the bus between the BMC and the PowerPC host)
>> descriptions, and USB support.
>>
>> The following changes since commit 7daf201d7fe8334e2d2364d4e8ed3394ec9af819:
>>
>> Linux 4.18-rc2 (2018-06-24 20:54:29 +0800)
>>
>> are available in the Git repository at:
>>
>> git://git.kernel.org/pub/scm/linux/kernel/git/joel/aspeed.git
>> tags/aspeed-4.19-devicetree
>>
>> for you to fetch changes up to 19617a35b08379b3e6da0a4ee0d2d62645bf1b9e:
>>
>> ARM: dts: aspeed: Add Power9 CFAM description (2018-07-25 17:43:36 +0930)
>>
>> ----------------------------------------------------------------
>> ASPEED device tree updates for 4.19
>>
>> - New support for the ASPEED USB host controller and USB vhub (device)
>> support
>>
>> - Descriptions for the FSI bus and hardware inside the Power8/Power9
>> processors that are accessed over this bus. This includes the ColdFire
>> processor that is part of the ASPEED SoC and is used to offload the FSI
>> bit-banging
>
> I had some feedback on one of these patches, it should be reworked before we
> merge it. See separate email as reply to the actual patch.
>
> If you prefer, you can send a branch without that contents for merging now, so
> the rest of it isn't held up.
No worries. I have sent a pull request without the FSI changes.
Thanks for taking a close look!
Cheers,
Joel
^ permalink raw reply
* [GIT PULL] ARM: aspeed: defconfig changes for 4.19
From: Joel Stanley @ 2018-07-27 1:17 UTC (permalink / raw)
To: linux-aspeed
In-Reply-To: <20180726192532.emdjzyaekz74ldqf@localhost>
On 27 July 2018 at 04:55, Olof Johansson <olof@lixom.net> wrote:
> On Thu, Jul 26, 2018 at 02:03:11PM +0930, Joel Stanley wrote:
>> Hello ARM Maintainers,
>>
>> Here are the ASPEED defconfig updates for 4.19. It's been a while since I sent
>> one, so this contains a lot of goodies.
>>
>> The following changes since commit 021c91791a5e7e85c567452f1be3e4c2c6cb6063:
>>
>> Linux 4.18-rc3 (2018-07-01 16:04:53 -0700)
>>
>> are available in the Git repository at:
>>
>> git://git.kernel.org/pub/scm/linux/kernel/git/joel/aspeed.git
>> tags/aspeed-4.19-defconfig
>>
>> for you to fetch changes up to 90708adcd3ead258d4c2185da6acbe748eb0cc32:
>>
>> ARM: config: aspeed: Enable new FSI drivers (2018-07-26 13:27:27 +0930)
>>
>> ----------------------------------------------------------------
>> ASPEED defconfig updates for 4.19
>>
>> - Refresh the multi ARMv5 defconfig, and add AST2400 related drivers
>>
>> - Enable new ASPEED hardware that we've merged in the past few cycles.
>> There are about 14 different drivers since we last refreshed the
>> defconfig
>>
>> - Turn on features required by systemd, and other bits of OpenBMC
>> userspace
>>
>> - Enable security related options
>
> Merged, two minor pieces of feedback:
>
> 1) I think you've cut-and-pasted the pull request into a mailer that line
> breaks between the URL and the tag. That's fine, but it means two copy and
> pastes at my end. Feel free to add a " \" at the end of the first line when
> you remember. We usually don't ask people to do it, it's purely for extra
> credit. :)
Ah, sorry! I'll try to sort that out in the future.
>
> 2) Please try to get these merge requests in before -rc6, or send what you have
> at that time even if you end up following up with just a few more patches
> later.
Thanks for the reminder. This was a little later than I intended.
^ permalink raw reply
* [PATCH 5/5] arm: dts: aspeed: Add power9 CFAM dtsi and use it on OpenPower P9 machines
From: Benjamin Herrenschmidt @ 2018-07-26 23:55 UTC (permalink / raw)
To: linux-aspeed
In-Reply-To: <CAOesGMjAGb7T-89C3eYPf7zsPYSp=7CVx756PNngs73ue27Jhw@mail.gmail.com>
On Thu, 2018-07-26 at 21:50 +0200, Olof Johansson wrote:
>
> > +++ b/arch/arm/boot/dts/ibm-power9-cfam.dtsi
> > @@ -0,0 +1,104 @@
> > +// SPDX-License-Identifier: GPL-2.0+
> > +// Copyright 2018 IBM Corp
> > +
> > +#define __MAKE_LABEL(p,i) p##i
> > +#define _MAKE_LABEL(p,i) __MAKE_LABEL(p,i)
> > +#define HUB_LABEL _MAKE_LABEL(fsi_hub,CFAM_CHIP_ID)
> > +#define I2C_LABEL(n) _MAKE_LABEL(_MAKE_LABEL(cfam,CFAM_CHIP_ID),_i2c##n)
>
> This is a red flag with respect to obscurity. It's a magic dtsi file
> that has to be included at the right spot in the dts file or things
> will break horribly -- we really try to avoid those kind of
> constructs.
I agree it sucks :-) I couldn't find any other way though. dtc seems to
be extremely limited in its ability to override node content. See
below.
> Also, you use it by passing in a predefined CHIP_ID, so you have
> #define / #include / #undef. And on top of that you have open-coded
> actual stable naming references to nodes that can't be found because
> they're created by macros.
Correct, as I said, I couldn't find a different way to do it.
> I know you want this to instantiate a bunch of boilerplate, so if you
> want to do that, maybe you'd be better off having this file just
> define a couple of macros, and then instantiate the actual subtrees
> with that macro (the macro can then take the chipid).
I'm not sure what you mean... the entire subtree being one giant macro
? That sounds scary ... I'll describe what I'm trying to do below.
> That way you can
> still expose the same label-making macros in the locations where you
> setup the stable naming. Much less cognitive load on someone trying to
> read it and figure out just what's being instantiated where, and what
> nodes the i2c<#> aliases are referring to.
I'm sorry, I don't quite get what you suggest...
>
> Also:
>
> > +/ {
> > + aliases {
> > + i2c100 = &cfam0_i2c0;
> > + i2c101 = &cfam0_i2c1;
> > + i2c102 = &cfam0_i2c2;
> > + i2c103 = &cfam0_i2c3;
> > + i2c104 = &cfam0_i2c4;
> > + i2c105 = &cfam0_i2c5;
> > + i2c106 = &cfam0_i2c6;
> > + i2c107 = &cfam0_i2c7;
> > + i2c108 = &cfam0_i2c8;
> > + i2c109 = &cfam0_i2c9;
> > + i2c110 = &cfam0_i2c10;
> > + i2c111 = &cfam0_i2c11;
> > + i2c112 = &cfam0_i2c12;
> > + i2c113 = &cfam0_i2c13;
> > + i2c114 = &cfam0_i2c14;
> > + i2c200 = &cfam1_i2c0;
> > + i2c201 = &cfam1_i2c1;
> > + i2c202 = &cfam1_i2c2;
> > + i2c203 = &cfam1_i2c3;
> > + i2c204 = &cfam1_i2c4;
> > + i2c205 = &cfam1_i2c5;
> > + i2c206 = &cfam1_i2c6;
> > + i2c207 = &cfam1_i2c7;
> > + i2c208 = &cfam1_i2c8;
> > + i2c209 = &cfam1_i2c9;
> > + i2c210 = &cfam1_i2c10;
> > + i2c211 = &cfam1_i2c11;
> > + i2c212 = &cfam1_i2c12;
> > + i2c213 = &cfam1_i2c13;
> > + i2c214 = &cfam1_i2c14;
>
> This is... unconventional. Fixed mapping but up at a high bus range
> such that it won't conflict with other SoC busses?
Yes. I'll describe the topology below.
> Just make your tools figure out what bus to use with sysfs or DT
> entries instead, much less of a hack. We've done these things to IRQs
> and GPIOs in the past, and it's a pain to clean up later.
Well, fixing the tools is ... hard and fixing the users of those tools
harder. Chances are people are just going to keep out of tree kernel
hacks rather than fixing their tools if that's what it gets to but we
can hope...
Ideally we could try creating a bunch of udev scripts that create named
symlinks for those i2c busses but that will take time. Anyway, here's
what we are trying to do:
So you may remember from your days at IBM :-) The FSI bus is the
service interface to the POWER chips (a bit like PECI on x86 I think).
It also connect to things like memory buffer chips etc...
This is the view of the system from the perspective of the BMC
management controller (or the FSP, same deal).
The BMC is typically master of one FSI bus to one chip, let's say P9
for now, and can access more chips via cascaded FSI masters (aka FSI
"hubs") in that chip. Those secondary chips can themselves have hubs to
other chips etc...
So we want a dtsi file that represent a "chip". ie, a power9.dtsi, a
power8.dtsi, a centaur.dtsi (centaur is the mem buffer) etc... There's
a while bunch of things in such a chip, the current patch only has a
portion of what will eventually be there. Via FSI we can access the OCC
thermal control processor, the i2c masters, the XSCOM engine, the scan
engine, etc. etc... Each of these will bind with drivers in the BMC
kernel.
And we want the top-level "system" file to instanciate them all in the
right places to represent a given system topology.
In addition, we do need some properties to uniquely identify a
chip/socket in the system in a stable manner. If anything to provide
meaningful data to the user, but also to cross-match with corresponding
fans etc...
Originally, I thought I could just use the DT "override" constructs to
do something like:
&bmc_fsi {
#include "power9.dtsi"
}
But then I couldn't find a way to then add the chip id to power9. dts
can't do things like
&bmc_fsi/cfam at 0 {
chip-id = <...>;
};
It seems.
I tried without using the bmc_fsi label, using a full path but that
failed too, dtc thought I was trying to create a duplicate node rather
than appending to an existing one.
Then I need to add another chip to one of the hub legs. Here too, how ?
I can have power9.dtsi define labels for its hub legs but then it will
clash when I bring in the second one...
So even ignoring the business with the i2c aliases, I'm a bit stuck.
I didn't quite get what you proposed with using the macros differently,
any chance you can throw a small example ? I'm hoping it doesn't
involve having the entire power9 chip content be a giant macro :-)
Cheers,
Ben.
^ permalink raw reply
* [GIT PULL] ARM: aspeed: dts changes for 4.19
From: Olof Johansson @ 2018-07-26 19:51 UTC (permalink / raw)
To: linux-aspeed
In-Reply-To: <CACPK8Xeu2JAVBTyiUtyso1egbe3X+r5a--Cx30YDJV-JmK==DQ@mail.gmail.com>
Hi,
On Thu, Jul 26, 2018 at 02:00:49PM +0930, Joel Stanley wrote:
> Hello ARM Maintainers,
>
> Here are the ASPEED device tree updates for 4.19. No new machines this time
> around, but we now have some FSI (the bus between the BMC and the PowerPC host)
> descriptions, and USB support.
>
> The following changes since commit 7daf201d7fe8334e2d2364d4e8ed3394ec9af819:
>
> Linux 4.18-rc2 (2018-06-24 20:54:29 +0800)
>
> are available in the Git repository at:
>
> git://git.kernel.org/pub/scm/linux/kernel/git/joel/aspeed.git
> tags/aspeed-4.19-devicetree
>
> for you to fetch changes up to 19617a35b08379b3e6da0a4ee0d2d62645bf1b9e:
>
> ARM: dts: aspeed: Add Power9 CFAM description (2018-07-25 17:43:36 +0930)
>
> ----------------------------------------------------------------
> ASPEED device tree updates for 4.19
>
> - New support for the ASPEED USB host controller and USB vhub (device)
> support
>
> - Descriptions for the FSI bus and hardware inside the Power8/Power9
> processors that are accessed over this bus. This includes the ColdFire
> processor that is part of the ASPEED SoC and is used to offload the FSI
> bit-banging
I had some feedback on one of these patches, it should be reworked before we
merge it. See separate email as reply to the actual patch.
If you prefer, you can send a branch without that contents for merging now, so
the rest of it isn't held up.
-Olof
^ permalink raw reply
* [PATCH 5/5] arm: dts: aspeed: Add power9 CFAM dtsi and use it on OpenPower P9 machines
From: Olof Johansson @ 2018-07-26 19:50 UTC (permalink / raw)
To: linux-aspeed
In-Reply-To: <20180724042406.15374-6-benh@kernel.crashing.org>
Hi,
I came across this patch as part of Joel's pull request, so this is
somewhat high latency review that I guess nobody else cared to do:
On Tue, Jul 24, 2018 at 6:24 AM, Benjamin Herrenschmidt
<benh@kernel.crashing.org> wrote:
> This provides proper chip IDs but also adds the various sub-devices
> necessary for the future OCC driver among other. All the added nodes
> comply with the existing upstream FSI bindings.
>
> Signed-off-by: Benjamin Herrenschmidt <benh@kernel.crashing.org>
> ---
> arch/arm/boot/dts/aspeed-bmc-opp-lanyang.dts | 1 +
> arch/arm/boot/dts/aspeed-bmc-opp-romulus.dts | 2 +
> .../boot/dts/aspeed-bmc-opp-witherspoon.dts | 2 +
> arch/arm/boot/dts/aspeed-bmc-opp-zaius.dts | 2 +
> arch/arm/boot/dts/ibm-power9-cfam.dtsi | 104 ++++++++++++++++++
> arch/arm/boot/dts/ibm-power9-dual.dtsi | 58 ++++++++++
> 6 files changed, 169 insertions(+)
> create mode 100644 arch/arm/boot/dts/ibm-power9-cfam.dtsi
> create mode 100644 arch/arm/boot/dts/ibm-power9-dual.dtsi
>
> diff --git a/arch/arm/boot/dts/ibm-power9-cfam.dtsi b/arch/arm/boot/dts/ibm-power9-cfam.dtsi
> new file mode 100644
> index 000000000000..5bda517f93cc
> --- /dev/null
> +++ b/arch/arm/boot/dts/ibm-power9-cfam.dtsi
> @@ -0,0 +1,104 @@
> +// SPDX-License-Identifier: GPL-2.0+
> +// Copyright 2018 IBM Corp
> +
> +#define __MAKE_LABEL(p,i) p##i
> +#define _MAKE_LABEL(p,i) __MAKE_LABEL(p,i)
> +#define HUB_LABEL _MAKE_LABEL(fsi_hub,CFAM_CHIP_ID)
> +#define I2C_LABEL(n) _MAKE_LABEL(_MAKE_LABEL(cfam,CFAM_CHIP_ID),_i2c##n)
This is a red flag with respect to obscurity. It's a magic dtsi file
that has to be included at the right spot in the dts file or things
will break horribly -- we really try to avoid those kind of
constructs.
Also, you use it by passing in a predefined CHIP_ID, so you have
#define / #include / #undef. And on top of that you have open-coded
actual stable naming references to nodes that can't be found because
they're created by macros.
I know you want this to instantiate a bunch of boilerplate, so if you
want to do that, maybe you'd be better off having this file just
define a couple of macros, and then instantiate the actual subtrees
with that macro (the macro can then take the chipid). That way you can
still expose the same label-making macros in the locations where you
setup the stable naming. Much less cognitive load on someone trying to
read it and figure out just what's being instantiated where, and what
nodes the i2c<#> aliases are referring to.
Also:
> +/ {
> + aliases {
> + i2c100 = &cfam0_i2c0;
> + i2c101 = &cfam0_i2c1;
> + i2c102 = &cfam0_i2c2;
> + i2c103 = &cfam0_i2c3;
> + i2c104 = &cfam0_i2c4;
> + i2c105 = &cfam0_i2c5;
> + i2c106 = &cfam0_i2c6;
> + i2c107 = &cfam0_i2c7;
> + i2c108 = &cfam0_i2c8;
> + i2c109 = &cfam0_i2c9;
> + i2c110 = &cfam0_i2c10;
> + i2c111 = &cfam0_i2c11;
> + i2c112 = &cfam0_i2c12;
> + i2c113 = &cfam0_i2c13;
> + i2c114 = &cfam0_i2c14;
> + i2c200 = &cfam1_i2c0;
> + i2c201 = &cfam1_i2c1;
> + i2c202 = &cfam1_i2c2;
> + i2c203 = &cfam1_i2c3;
> + i2c204 = &cfam1_i2c4;
> + i2c205 = &cfam1_i2c5;
> + i2c206 = &cfam1_i2c6;
> + i2c207 = &cfam1_i2c7;
> + i2c208 = &cfam1_i2c8;
> + i2c209 = &cfam1_i2c9;
> + i2c210 = &cfam1_i2c10;
> + i2c211 = &cfam1_i2c11;
> + i2c212 = &cfam1_i2c12;
> + i2c213 = &cfam1_i2c13;
> + i2c214 = &cfam1_i2c14;
This is... unconventional. Fixed mapping but up at a high bus range
such that it won't conflict with other SoC busses?
Just make your tools figure out what bus to use with sysfs or DT
entries instead, much less of a hack. We've done these things to IRQs
and GPIOs in the past, and it's a pain to clean up later.
-Olof
^ permalink raw reply
* [GIT PULL] ARM: aspeed: defconfig changes for 4.19
From: Olof Johansson @ 2018-07-26 19:25 UTC (permalink / raw)
To: linux-aspeed
In-Reply-To: <CACPK8XdcAANiSwsvCnu249W8QcFfjP616ce-YOW=UohgpTHtVw@mail.gmail.com>
On Thu, Jul 26, 2018 at 02:03:11PM +0930, Joel Stanley wrote:
> Hello ARM Maintainers,
>
> Here are the ASPEED defconfig updates for 4.19. It's been a while since I sent
> one, so this contains a lot of goodies.
>
> The following changes since commit 021c91791a5e7e85c567452f1be3e4c2c6cb6063:
>
> Linux 4.18-rc3 (2018-07-01 16:04:53 -0700)
>
> are available in the Git repository at:
>
> git://git.kernel.org/pub/scm/linux/kernel/git/joel/aspeed.git
> tags/aspeed-4.19-defconfig
>
> for you to fetch changes up to 90708adcd3ead258d4c2185da6acbe748eb0cc32:
>
> ARM: config: aspeed: Enable new FSI drivers (2018-07-26 13:27:27 +0930)
>
> ----------------------------------------------------------------
> ASPEED defconfig updates for 4.19
>
> - Refresh the multi ARMv5 defconfig, and add AST2400 related drivers
>
> - Enable new ASPEED hardware that we've merged in the past few cycles.
> There are about 14 different drivers since we last refreshed the
> defconfig
>
> - Turn on features required by systemd, and other bits of OpenBMC
> userspace
>
> - Enable security related options
Merged, two minor pieces of feedback:
1) I think you've cut-and-pasted the pull request into a mailer that line
breaks between the URL and the tag. That's fine, but it means two copy and
pastes at my end. Feel free to add a " \" at the end of the first line when
you remember. We usually don't ask people to do it, it's purely for extra
credit. :)
2) Please try to get these merge requests in before -rc6, or send what you have
at that time even if you end up following up with just a few more patches
later.
-Olof
^ permalink raw reply
* [GIT PULL] ARM: aspeed: defconfig changes for 4.19
From: Joel Stanley @ 2018-07-26 4:33 UTC (permalink / raw)
To: linux-aspeed
Hello ARM Maintainers,
Here are the ASPEED defconfig updates for 4.19. It's been a while since I sent
one, so this contains a lot of goodies.
The following changes since commit 021c91791a5e7e85c567452f1be3e4c2c6cb6063:
Linux 4.18-rc3 (2018-07-01 16:04:53 -0700)
are available in the Git repository at:
git://git.kernel.org/pub/scm/linux/kernel/git/joel/aspeed.git
tags/aspeed-4.19-defconfig
for you to fetch changes up to 90708adcd3ead258d4c2185da6acbe748eb0cc32:
ARM: config: aspeed: Enable new FSI drivers (2018-07-26 13:27:27 +0930)
----------------------------------------------------------------
ASPEED defconfig updates for 4.19
- Refresh the multi ARMv5 defconfig, and add AST2400 related drivers
- Enable new ASPEED hardware that we've merged in the past few cycles.
There are about 14 different drivers since we last refreshed the
defconfig
- Turn on features required by systemd, and other bits of OpenBMC
userspace
- Enable security related options
----------------------------------------------------------------
Benjamin Herrenschmidt (2):
arm: configs: Add USB gadget to Aspeed G4 defconfig
arm: configs: Add USB gadget to Aspeed G5 defconfig
Joel Stanley (4):
ARM: config: aspeed: Update defconfig
ARM: config: multi_v5: Refresh configuration
ARM: config: multi_v5: Enable ASPEED drivers
ARM: config: aspeed: Enable new FSI drivers
arch/arm/configs/aspeed_g4_defconfig | 113 ++++++++++++++++++++++++++++-----
arch/arm/configs/aspeed_g5_defconfig | 117 +++++++++++++++++++++++++++++------
arch/arm/configs/multi_v5_defconfig | 30 +++++----
3 files changed, 213 insertions(+), 47 deletions(-)
^ permalink raw reply
* [GIT PULL] ARM: aspeed: dts changes for 4.19
From: Joel Stanley @ 2018-07-26 4:30 UTC (permalink / raw)
To: linux-aspeed
Hello ARM Maintainers,
Here are the ASPEED device tree updates for 4.19. No new machines this time
around, but we now have some FSI (the bus between the BMC and the PowerPC host)
descriptions, and USB support.
The following changes since commit 7daf201d7fe8334e2d2364d4e8ed3394ec9af819:
Linux 4.18-rc2 (2018-06-24 20:54:29 +0800)
are available in the Git repository at:
git://git.kernel.org/pub/scm/linux/kernel/git/joel/aspeed.git
tags/aspeed-4.19-devicetree
for you to fetch changes up to 19617a35b08379b3e6da0a4ee0d2d62645bf1b9e:
ARM: dts: aspeed: Add Power9 CFAM description (2018-07-25 17:43:36 +0930)
----------------------------------------------------------------
ASPEED device tree updates for 4.19
- New support for the ASPEED USB host controller and USB vhub (device)
support
- Descriptions for the FSI bus and hardware inside the Power8/Power9
processors that are accessed over this bus. This includes the ColdFire
processor that is part of the ASPEED SoC and is used to offload the FSI
bit-banging
- Small fixes:
* pwm/tach clock
* Romulus framebuffer location
----------------------------------------------------------------
Benjamin Herrenschmidt (11):
ARM: dts: aspeed: Add G4 USB pinmux
ARM: dts: aspeed: Add G5 USB host pinmux
ARM: dts: aspeed: Add G4 USB Virtual Hub
ARM: dts: aspeed: Add G5 USB Virtual Hub
ARM: dts: aspeed: Enable vhub on port A of AST2500 EVB
ARM: dts: aspeed: Fix Romulus VGA frame buffer
ARM: dts: aspeed: Add coprocessor interrupt controller
ARM: dts: aspeed: Romulus system can use coprocessor for FSI
ARM: dts: aspeed: Palmetto system can use coprocessor for FSI
ARM: dts: aspeed: Add Power8 CFAM description for use by Palmetto
ARM: dts: aspeed: Add Power9 CFAM description
Lei YU (1):
ARM: dts: aspeed: Use 24MHz fixed clock for pwm
arch/arm/boot/dts/aspeed-ast2500-evb.dts | 14 +--
arch/arm/boot/dts/aspeed-bmc-opp-lanyang.dts | 1 +
arch/arm/boot/dts/aspeed-bmc-opp-palmetto.dts | 38 +++++++--
arch/arm/boot/dts/aspeed-bmc-opp-romulus.dts | 17 +++-
arch/arm/boot/dts/aspeed-bmc-opp-witherspoon.dts | 2 +
arch/arm/boot/dts/aspeed-bmc-opp-zaius.dts | 2 +
arch/arm/boot/dts/aspeed-g4.dtsi | 26 +++++-
arch/arm/boot/dts/aspeed-g5.dtsi | 34 +++++++-
arch/arm/boot/dts/ibm-power8-cfam.dtsi | 31 +++++++
arch/arm/boot/dts/ibm-power9-cfam.dtsi | 104 +++++++++++++++++++++++
arch/arm/boot/dts/ibm-power9-dual.dtsi | 57 +++++++++++++
11 files changed, 305 insertions(+), 21 deletions(-)
create mode 100644 arch/arm/boot/dts/ibm-power8-cfam.dtsi
create mode 100644 arch/arm/boot/dts/ibm-power9-cfam.dtsi
create mode 100644 arch/arm/boot/dts/ibm-power9-dual.dtsi
^ permalink raw reply
* [PATCH i2c-next v2] i2c: aspeed: Add an explicit type casting for *get_clk_reg_val
From: Jae Hyun Yoo @ 2018-07-24 20:36 UTC (permalink / raw)
To: linux-aspeed
This commit fixes this sparse warning:
drivers/i2c/busses/i2c-aspeed.c:875:38: warning: incorrect type in assignment (different modifiers)
drivers/i2c/busses/i2c-aspeed.c:875:38: expected unsigned int ( *get_clk_reg_val )( ... )
drivers/i2c/busses/i2c-aspeed.c:875:38: got void const *const data
Signed-off-by: Jae Hyun Yoo <jae.hyun.yoo@linux.intel.com>
---
Changes since v1:
- Fixed title and added Reported-by tag.
drivers/i2c/busses/i2c-aspeed.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/i2c/busses/i2c-aspeed.c b/drivers/i2c/busses/i2c-aspeed.c
index efb89422d496..a4f956c6d567 100644
--- a/drivers/i2c/busses/i2c-aspeed.c
+++ b/drivers/i2c/busses/i2c-aspeed.c
@@ -872,7 +872,7 @@ static int aspeed_i2c_probe_bus(struct platform_device *pdev)
if (!match)
bus->get_clk_reg_val = aspeed_i2c_24xx_get_clk_reg_val;
else
- bus->get_clk_reg_val = match->data;
+ bus->get_clk_reg_val = (u32 (*)(u32))match->data;
/* Initialize the I2C adapter */
spin_lock_init(&bus->lock);
--
2.18.0
^ permalink raw reply related
* [PATCH v2] i2c: aspeed: Handle master/slave combined irq events properly
From: Jae Hyun Yoo @ 2018-07-24 17:31 UTC (permalink / raw)
To: linux-aspeed
In most of cases, interrupt bits are set one by one but there are
also a lot of other cases that Aspeed I2C IP sends multiple
interrupt bits with combining master and slave events using a
single interrupt call. It happens much more in multi-master
environment than single-master. For an example, when master is
waiting for a NORMAL_STOP interrupt in its MASTER_STOP state,
SLAVE_MATCH and RX_DONE interrupts could come along with the
NORMAL_STOP in case of an another master immediately sends data
just after acquiring the bus. In this case, the NORMAL_STOP
interrupt should be handled by master_irq and the SLAVE_MATCH and
RX_DONE interrupts should be handled by slave_irq. This commit
modifies irq hadling logic to handle the master/slave combined
events properly.
Changes since v1:
- Fixed a grammer issue in commit message.
- Added a missing line feed character into a message printing.
Signed-off-by: Jae Hyun Yoo <jae.hyun.yoo@linux.intel.com>
---
drivers/i2c/busses/i2c-aspeed.c | 135 ++++++++++++++++++--------------
1 file changed, 75 insertions(+), 60 deletions(-)
diff --git a/drivers/i2c/busses/i2c-aspeed.c b/drivers/i2c/busses/i2c-aspeed.c
index efb89422d496..75431e305073 100644
--- a/drivers/i2c/busses/i2c-aspeed.c
+++ b/drivers/i2c/busses/i2c-aspeed.c
@@ -82,6 +82,11 @@
#define ASPEED_I2CD_INTR_RX_DONE BIT(2)
#define ASPEED_I2CD_INTR_TX_NAK BIT(1)
#define ASPEED_I2CD_INTR_TX_ACK BIT(0)
+#define ASPEED_I2CD_INTR_ERRORS \
+ (ASPEED_I2CD_INTR_SDA_DL_TIMEOUT | \
+ ASPEED_I2CD_INTR_SCL_TIMEOUT | \
+ ASPEED_I2CD_INTR_ABNORMAL | \
+ ASPEED_I2CD_INTR_ARBIT_LOSS)
#define ASPEED_I2CD_INTR_ALL \
(ASPEED_I2CD_INTR_SDA_DL_TIMEOUT | \
ASPEED_I2CD_INTR_BUS_RECOVER_DONE | \
@@ -150,6 +155,7 @@ struct aspeed_i2c_bus {
int cmd_err;
/* Protected only by i2c_lock_bus */
int master_xfer_result;
+ u32 irq_status;
#if IS_ENABLED(CONFIG_I2C_SLAVE)
struct i2c_client *slave;
enum aspeed_i2c_slave_state slave_state;
@@ -229,36 +235,30 @@ static int aspeed_i2c_recover_bus(struct aspeed_i2c_bus *bus)
#if IS_ENABLED(CONFIG_I2C_SLAVE)
static bool aspeed_i2c_slave_irq(struct aspeed_i2c_bus *bus)
{
- u32 command, irq_status, status_ack = 0;
+ u32 command, status_ack = 0;
struct i2c_client *slave = bus->slave;
- bool irq_handled = true;
u8 value;
- if (!slave) {
- irq_handled = false;
- goto out;
- }
+ if (!slave)
+ return false;
command = readl(bus->base + ASPEED_I2C_CMD_REG);
- irq_status = readl(bus->base + ASPEED_I2C_INTR_STS_REG);
/* Slave was requested, restart state machine. */
- if (irq_status & ASPEED_I2CD_INTR_SLAVE_MATCH) {
+ if (bus->irq_status & ASPEED_I2CD_INTR_SLAVE_MATCH) {
status_ack |= ASPEED_I2CD_INTR_SLAVE_MATCH;
bus->slave_state = ASPEED_I2C_SLAVE_START;
}
/* Slave is not currently active, irq was for someone else. */
- if (bus->slave_state == ASPEED_I2C_SLAVE_STOP) {
- irq_handled = false;
- goto out;
- }
+ if (bus->slave_state == ASPEED_I2C_SLAVE_STOP)
+ return false;
dev_dbg(bus->dev, "slave irq status 0x%08x, cmd 0x%08x\n",
- irq_status, command);
+ bus->irq_status, command);
/* Slave was sent something. */
- if (irq_status & ASPEED_I2CD_INTR_RX_DONE) {
+ if (bus->irq_status & ASPEED_I2CD_INTR_RX_DONE) {
value = readl(bus->base + ASPEED_I2C_BYTE_BUF_REG) >> 8;
/* Handle address frame. */
if (bus->slave_state == ASPEED_I2C_SLAVE_START) {
@@ -273,28 +273,29 @@ static bool aspeed_i2c_slave_irq(struct aspeed_i2c_bus *bus)
}
/* Slave was asked to stop. */
- if (irq_status & ASPEED_I2CD_INTR_NORMAL_STOP) {
+ if (bus->irq_status & ASPEED_I2CD_INTR_NORMAL_STOP) {
status_ack |= ASPEED_I2CD_INTR_NORMAL_STOP;
bus->slave_state = ASPEED_I2C_SLAVE_STOP;
}
- if (irq_status & ASPEED_I2CD_INTR_TX_NAK) {
+ if (bus->irq_status & ASPEED_I2CD_INTR_TX_NAK) {
status_ack |= ASPEED_I2CD_INTR_TX_NAK;
bus->slave_state = ASPEED_I2C_SLAVE_STOP;
}
+ if (bus->irq_status & ASPEED_I2CD_INTR_TX_ACK) {
+ status_ack |= ASPEED_I2CD_INTR_TX_ACK;
+ }
switch (bus->slave_state) {
case ASPEED_I2C_SLAVE_READ_REQUESTED:
- if (irq_status & ASPEED_I2CD_INTR_TX_ACK)
+ if (bus->irq_status & ASPEED_I2CD_INTR_TX_ACK)
dev_err(bus->dev, "Unexpected ACK on read request.\n");
bus->slave_state = ASPEED_I2C_SLAVE_READ_PROCESSED;
-
i2c_slave_event(slave, I2C_SLAVE_READ_REQUESTED, &value);
writel(value, bus->base + ASPEED_I2C_BYTE_BUF_REG);
writel(ASPEED_I2CD_S_TX_CMD, bus->base + ASPEED_I2C_CMD_REG);
break;
case ASPEED_I2C_SLAVE_READ_PROCESSED:
- status_ack |= ASPEED_I2CD_INTR_TX_ACK;
- if (!(irq_status & ASPEED_I2CD_INTR_TX_ACK))
+ if (!(bus->irq_status & ASPEED_I2CD_INTR_TX_ACK))
dev_err(bus->dev,
"Expected ACK after processed read.\n");
i2c_slave_event(slave, I2C_SLAVE_READ_PROCESSED, &value);
@@ -317,14 +318,8 @@ static bool aspeed_i2c_slave_irq(struct aspeed_i2c_bus *bus)
break;
}
- if (status_ack != irq_status)
- dev_err(bus->dev,
- "irq handled != irq. expected %x, but was %x\n",
- irq_status, status_ack);
- writel(status_ack, bus->base + ASPEED_I2C_INTR_STS_REG);
-
-out:
- return irq_handled;
+ bus->irq_status ^= status_ack;
+ return !bus->irq_status;
}
#endif /* CONFIG_I2C_SLAVE */
@@ -382,19 +377,19 @@ static int aspeed_i2c_is_irq_error(u32 irq_status)
static bool aspeed_i2c_master_irq(struct aspeed_i2c_bus *bus)
{
- u32 irq_status, status_ack = 0, command = 0;
+ u32 status_ack = 0, command = 0;
struct i2c_msg *msg;
u8 recv_byte;
int ret;
- irq_status = readl(bus->base + ASPEED_I2C_INTR_STS_REG);
- /* Ack all interrupt bits. */
- writel(irq_status, bus->base + ASPEED_I2C_INTR_STS_REG);
-
- if (irq_status & ASPEED_I2CD_INTR_BUS_RECOVER_DONE) {
+ if (bus->irq_status & ASPEED_I2CD_INTR_BUS_RECOVER_DONE) {
bus->master_state = ASPEED_I2C_MASTER_INACTIVE;
status_ack |= ASPEED_I2CD_INTR_BUS_RECOVER_DONE;
goto out_complete;
+ } else {
+ /* Master is not currently active, irq was for someone else. */
+ if (bus->master_state == ASPEED_I2C_MASTER_INACTIVE)
+ goto out_no_complete;
}
/*
@@ -402,20 +397,23 @@ static bool aspeed_i2c_master_irq(struct aspeed_i2c_bus *bus)
* should clear the command queue effectively taking us back to the
* INACTIVE state.
*/
- ret = aspeed_i2c_is_irq_error(irq_status);
- if (ret < 0) {
+ ret = aspeed_i2c_is_irq_error(bus->irq_status);
+ if (ret) {
dev_dbg(bus->dev, "received error interrupt: 0x%08x\n",
- irq_status);
+ bus->irq_status);
bus->cmd_err = ret;
bus->master_state = ASPEED_I2C_MASTER_INACTIVE;
+ status_ack |= (bus->irq_status & ASPEED_I2CD_INTR_ERRORS);
goto out_complete;
}
/* We are in an invalid state; reset bus to a known state. */
if (!bus->msgs) {
- dev_err(bus->dev, "bus in unknown state\n");
+ dev_err(bus->dev, "bus in unknown state irq_status: 0x%x\n",
+ bus->irq_status);
bus->cmd_err = -EIO;
- if (bus->master_state != ASPEED_I2C_MASTER_STOP)
+ if (bus->master_state != ASPEED_I2C_MASTER_STOP &&
+ bus->master_state != ASPEED_I2C_MASTER_INACTIVE)
aspeed_i2c_do_stop(bus);
goto out_no_complete;
}
@@ -427,7 +425,13 @@ static bool aspeed_i2c_master_irq(struct aspeed_i2c_bus *bus)
* then update the state and handle the new state below.
*/
if (bus->master_state == ASPEED_I2C_MASTER_START) {
- if (unlikely(!(irq_status & ASPEED_I2CD_INTR_TX_ACK))) {
+ if (unlikely(!(bus->irq_status & ASPEED_I2CD_INTR_TX_ACK))) {
+ if (unlikely(!(bus->irq_status &
+ ASPEED_I2CD_INTR_TX_NAK))) {
+ bus->cmd_err = -ENXIO;
+ bus->master_state = ASPEED_I2C_MASTER_INACTIVE;
+ goto out_complete;
+ }
pr_devel("no slave present at %02x\n", msg->addr);
status_ack |= ASPEED_I2CD_INTR_TX_NAK;
bus->cmd_err = -ENXIO;
@@ -447,11 +451,12 @@ static bool aspeed_i2c_master_irq(struct aspeed_i2c_bus *bus)
switch (bus->master_state) {
case ASPEED_I2C_MASTER_TX:
- if (unlikely(irq_status & ASPEED_I2CD_INTR_TX_NAK)) {
+ if (unlikely(bus->irq_status & ASPEED_I2CD_INTR_TX_NAK)) {
dev_dbg(bus->dev, "slave NACKed TX\n");
status_ack |= ASPEED_I2CD_INTR_TX_NAK;
goto error_and_stop;
- } else if (unlikely(!(irq_status & ASPEED_I2CD_INTR_TX_ACK))) {
+ } else if (unlikely(!(bus->irq_status &
+ ASPEED_I2CD_INTR_TX_ACK))) {
dev_err(bus->dev, "slave failed to ACK TX\n");
goto error_and_stop;
}
@@ -470,11 +475,11 @@ static bool aspeed_i2c_master_irq(struct aspeed_i2c_bus *bus)
goto out_no_complete;
case ASPEED_I2C_MASTER_RX_FIRST:
/* RX may not have completed yet (only address cycle) */
- if (!(irq_status & ASPEED_I2CD_INTR_RX_DONE))
+ if (!(bus->irq_status & ASPEED_I2CD_INTR_RX_DONE))
goto out_no_complete;
/* fallthrough intended */
case ASPEED_I2C_MASTER_RX:
- if (unlikely(!(irq_status & ASPEED_I2CD_INTR_RX_DONE))) {
+ if (unlikely(!(bus->irq_status & ASPEED_I2CD_INTR_RX_DONE))) {
dev_err(bus->dev, "master failed to RX\n");
goto error_and_stop;
}
@@ -505,8 +510,11 @@ static bool aspeed_i2c_master_irq(struct aspeed_i2c_bus *bus)
}
goto out_no_complete;
case ASPEED_I2C_MASTER_STOP:
- if (unlikely(!(irq_status & ASPEED_I2CD_INTR_NORMAL_STOP))) {
- dev_err(bus->dev, "master failed to STOP\n");
+ if (unlikely(!(bus->irq_status &
+ ASPEED_I2CD_INTR_NORMAL_STOP))) {
+ dev_err(bus->dev,
+ "master failed to STOP irq_status:0x%x\n",
+ bus->irq_status);
bus->cmd_err = -EIO;
/* Do not STOP as we have already tried. */
} else {
@@ -518,7 +526,7 @@ static bool aspeed_i2c_master_irq(struct aspeed_i2c_bus *bus)
case ASPEED_I2C_MASTER_INACTIVE:
dev_err(bus->dev,
"master received interrupt 0x%08x, but is inactive\n",
- irq_status);
+ bus->irq_status);
bus->cmd_err = -EIO;
/* Do not STOP as we should be inactive. */
goto out_complete;
@@ -540,33 +548,40 @@ static bool aspeed_i2c_master_irq(struct aspeed_i2c_bus *bus)
bus->master_xfer_result = bus->msgs_index + 1;
complete(&bus->cmd_complete);
out_no_complete:
- if (irq_status != status_ack)
- dev_err(bus->dev,
- "irq handled != irq. expected 0x%08x, but was 0x%08x\n",
- irq_status, status_ack);
- return !!irq_status;
+ bus->irq_status ^= status_ack;
+ return !bus->irq_status;
}
static irqreturn_t aspeed_i2c_bus_irq(int irq, void *dev_id)
{
struct aspeed_i2c_bus *bus = dev_id;
- bool ret;
+ u32 irq_received;
spin_lock(&bus->lock);
+ irq_received = readl(bus->base + ASPEED_I2C_INTR_STS_REG);
+ bus->irq_status = irq_received;
#if IS_ENABLED(CONFIG_I2C_SLAVE)
- if (aspeed_i2c_slave_irq(bus)) {
- dev_dbg(bus->dev, "irq handled by slave.\n");
- ret = true;
- goto out;
+ if (bus->master_state != ASPEED_I2C_MASTER_INACTIVE) {
+ if (!aspeed_i2c_master_irq(bus))
+ aspeed_i2c_slave_irq(bus);
+ } else {
+ if (!aspeed_i2c_slave_irq(bus))
+ aspeed_i2c_master_irq(bus);
}
+#else
+ aspeed_i2c_master_irq(bus);
#endif /* CONFIG_I2C_SLAVE */
- ret = aspeed_i2c_master_irq(bus);
+ if (bus->irq_status)
+ dev_err(bus->dev,
+ "irq handled != irq. expected 0x%08x, but was 0x%08x\n",
+ irq_received, irq_received ^ bus->irq_status);
-out:
+ /* Ack all interrupt bits. */
+ writel(irq_received, bus->base + ASPEED_I2C_INTR_STS_REG);
spin_unlock(&bus->lock);
- return ret ? IRQ_HANDLED : IRQ_NONE;
+ return bus->irq_status ? IRQ_NONE : IRQ_HANDLED;
}
static int aspeed_i2c_master_xfer(struct i2c_adapter *adap,
--
2.18.0
^ permalink raw reply related
* [PATCH 5/5] fsi: Prevent multiple concurrent rescans
From: Benjamin Herrenschmidt @ 2018-07-24 5:05 UTC (permalink / raw)
To: linux-aspeed
In-Reply-To: <20180724050519.31920-1-benh@kernel.crashing.org>
The bus scanning process isn't terribly good at parallel attempts
at rescanning the same bus. Let's have a per-master mutex protecting
the scanning process.
Signed-off-by: Benjamin Herrenschmidt <benh@kernel.crashing.org>
---
drivers/fsi/fsi-core.c | 16 ++++++++++++++--
drivers/fsi/fsi-master.h | 2 ++
2 files changed, 16 insertions(+), 2 deletions(-)
diff --git a/drivers/fsi/fsi-core.c b/drivers/fsi/fsi-core.c
index dea5bd48acc5..2c31563fdcae 100644
--- a/drivers/fsi/fsi-core.c
+++ b/drivers/fsi/fsi-core.c
@@ -1203,8 +1203,14 @@ static void fsi_master_unscan(struct fsi_master *master)
int fsi_master_rescan(struct fsi_master *master)
{
+ int rc;
+
+ mutex_lock(&master->scan_lock);
fsi_master_unscan(master);
- return fsi_master_scan(master);
+ rc = fsi_master_scan(master);
+ mutex_unlock(&master->scan_lock);
+
+ return rc;
}
EXPORT_SYMBOL_GPL(fsi_master_rescan);
@@ -1240,6 +1246,7 @@ int fsi_master_register(struct fsi_master *master)
int rc;
struct device_node *np;
+ mutex_init(&master->scan_lock);
master->idx = ida_simple_get(&master_ida, 0, INT_MAX, GFP_KERNEL);
dev_set_name(&master->dev, "fsi%d", master->idx);
@@ -1264,8 +1271,11 @@ int fsi_master_register(struct fsi_master *master)
}
np = dev_of_node(&master->dev);
- if (!of_property_read_bool(np, "no-scan-on-init"))
+ if (!of_property_read_bool(np, "no-scan-on-init")) {
+ mutex_lock(&master->scan_lock);
fsi_master_scan(master);
+ mutex_unlock(&master->scan_lock);
+ }
return 0;
}
@@ -1278,7 +1288,9 @@ void fsi_master_unregister(struct fsi_master *master)
master->idx = -1;
}
+ mutex_lock(&master->scan_lock);
fsi_master_unscan(master);
+ mutex_unlock(&master->scan_lock);
device_unregister(&master->dev);
}
EXPORT_SYMBOL_GPL(fsi_master_unregister);
diff --git a/drivers/fsi/fsi-master.h b/drivers/fsi/fsi-master.h
index f653f75da7be..040a7d4cf717 100644
--- a/drivers/fsi/fsi-master.h
+++ b/drivers/fsi/fsi-master.h
@@ -18,6 +18,7 @@
#define DRIVERS_FSI_MASTER_H
#include <linux/device.h>
+#include <linux/mutex.h>
/* Various protocol delays */
#define FSI_ECHO_DELAY_CLOCKS 16 /* Number clocks for echo delay */
@@ -59,6 +60,7 @@ struct fsi_master {
int idx;
int n_links;
int flags;
+ struct mutex scan_lock;
int (*read)(struct fsi_master *, int link, uint8_t id,
uint32_t addr, void *val, size_t size);
int (*write)(struct fsi_master *, int link, uint8_t id,
--
2.17.1
^ permalink raw reply related
* [PATCH 4/5] fsi: Add cfam char devices
From: Benjamin Herrenschmidt @ 2018-07-24 5:05 UTC (permalink / raw)
To: linux-aspeed
In-Reply-To: <20180724050519.31920-1-benh@kernel.crashing.org>
This aims to deprecate the "raw" sysfs file used for directly
accessing the CFAM and instead use a char device like the
other sub drivers.
Since it reworks the slave creation code and adds a cfam device
type, we also use the opportunity to convert the attributes
to attribute groups and add a couple more.
Signed-off-by: Benjamin Herrenschmidt <benh@kernel.crashing.org>
---
drivers/fsi/fsi-core.c | 264 +++++++++++++++++++++++++++++++++--------
1 file changed, 213 insertions(+), 51 deletions(-)
diff --git a/drivers/fsi/fsi-core.c b/drivers/fsi/fsi-core.c
index faa1760a5a40..dea5bd48acc5 100644
--- a/drivers/fsi/fsi-core.c
+++ b/drivers/fsi/fsi-core.c
@@ -11,6 +11,11 @@
* 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.
+ *
+ * TODO:
+ * - Rework topology
+ * - s/chip_id/chip_loc
+ * - s/cfam/chip (cfam_id -> chip_id etc...)
*/
#include <linux/crc4.h>
@@ -21,6 +26,9 @@
#include <linux/of.h>
#include <linux/slab.h>
#include <linux/bitops.h>
+#include <linux/cdev.h>
+#include <linux/fs.h>
+#include <linux/uaccess.h>
#include "fsi-master.h"
@@ -78,8 +86,11 @@ static DEFINE_IDA(master_ida);
struct fsi_slave {
struct device dev;
struct fsi_master *master;
- int id;
- int link;
+ struct cdev cdev;
+ int cdev_idx;
+ int id; /* FSI address */
+ int link; /* FSI link# */
+ u32 cfam_id;
int chip_id;
uint32_t size; /* size of slave address space */
u8 t_send_delay;
@@ -607,29 +618,6 @@ static const struct bin_attribute fsi_slave_raw_attr = {
.write = fsi_slave_sysfs_raw_write,
};
-static ssize_t fsi_slave_sysfs_term_write(struct file *file,
- struct kobject *kobj, struct bin_attribute *attr,
- char *buf, loff_t off, size_t count)
-{
- struct fsi_slave *slave = to_fsi_slave(kobj_to_dev(kobj));
- struct fsi_master *master = slave->master;
-
- if (!master->term)
- return -ENODEV;
-
- master->term(master, slave->link, slave->id);
- return count;
-}
-
-static const struct bin_attribute fsi_slave_term_attr = {
- .attr = {
- .name = "term",
- .mode = 0200,
- },
- .size = 0,
- .write = fsi_slave_sysfs_term_write,
-};
-
static void fsi_slave_release(struct device *dev)
{
struct fsi_slave *slave = to_fsi_slave(dev);
@@ -682,6 +670,127 @@ static struct device_node *fsi_slave_find_of_node(struct fsi_master *master,
return NULL;
}
+static ssize_t cfam_read(struct file *filep, char __user *buf, size_t count,
+ loff_t *offset)
+{
+ struct fsi_slave *slave = filep->private_data;
+ size_t total_len, read_len;
+ loff_t off = *offset;
+ ssize_t rc;
+
+ if (off < 0)
+ return -EINVAL;
+
+ if (off > 0xffffffff || count > 0xffffffff || off + count > 0xffffffff)
+ return -EINVAL;
+
+ for (total_len = 0; total_len < count; total_len += read_len) {
+ __be32 data;
+
+ read_len = min_t(size_t, count, 4);
+ read_len -= off & 0x3;
+
+ rc = fsi_slave_read(slave, off, &data, read_len);
+ if (rc)
+ goto fail;
+ rc = copy_to_user(buf + total_len, &data, read_len);
+ if (rc) {
+ rc = -EFAULT;
+ goto fail;
+ }
+ off += read_len;
+ }
+ rc = count;
+ fail:
+ *offset = off;
+ return count;
+}
+
+static ssize_t cfam_write(struct file *filep, const char __user *buf,
+ size_t count, loff_t *offset)
+{
+ struct fsi_slave *slave = filep->private_data;
+ size_t total_len, write_len;
+ loff_t off = *offset;
+ ssize_t rc;
+
+
+ if (off < 0)
+ return -EINVAL;
+
+ if (off > 0xffffffff || count > 0xffffffff || off + count > 0xffffffff)
+ return -EINVAL;
+
+ for (total_len = 0; total_len < count; total_len += write_len) {
+ __be32 data;
+
+ write_len = min_t(size_t, count, 4);
+ write_len -= off & 0x3;
+
+ rc = copy_from_user(&data, buf + total_len, write_len);
+ if (rc) {
+ rc = -EFAULT;
+ goto fail;
+ }
+ rc = fsi_slave_write(slave, off, &data, write_len);
+ if (rc)
+ goto fail;
+ off += write_len;
+ }
+ rc = count;
+ fail:
+ *offset = off;
+ return count;
+}
+
+static loff_t cfam_llseek(struct file *file, loff_t offset, int whence)
+{
+ switch (whence) {
+ case SEEK_CUR:
+ break;
+ case SEEK_SET:
+ file->f_pos = offset;
+ break;
+ default:
+ return -EINVAL;
+ }
+
+ return offset;
+}
+
+static int cfam_open(struct inode *inode, struct file *file)
+{
+ struct fsi_slave *slave = container_of(inode->i_cdev, struct fsi_slave, cdev);
+
+ file->private_data = slave;
+
+ return 0;
+}
+
+static const struct file_operations cfam_fops = {
+ .owner = THIS_MODULE,
+ .open = cfam_open,
+ .llseek = cfam_llseek,
+ .read = cfam_read,
+ .write = cfam_write,
+};
+
+static ssize_t send_term_store(struct device *dev,
+ struct device_attribute *attr,
+ const char *buf, size_t count)
+{
+ struct fsi_slave *slave = to_fsi_slave(dev);
+ struct fsi_master *master = slave->master;
+
+ if (!master->term)
+ return -ENODEV;
+
+ master->term(master, slave->link, slave->id);
+ return count;
+}
+
+static DEVICE_ATTR_WO(send_term);
+
static ssize_t slave_send_echo_show(struct device *dev,
struct device_attribute *attr,
char *buf)
@@ -737,6 +846,52 @@ static ssize_t chip_id_show(struct device *dev,
static DEVICE_ATTR_RO(chip_id);
+static ssize_t cfam_id_show(struct device *dev,
+ struct device_attribute *attr,
+ char *buf)
+{
+ struct fsi_slave *slave = to_fsi_slave(dev);
+
+ return sprintf(buf, "0x%x\n", slave->cfam_id);
+}
+
+static DEVICE_ATTR_RO(cfam_id);
+
+static struct attribute *cfam_attr[] = {
+ &dev_attr_send_echo_delays.attr,
+ &dev_attr_chip_id.attr,
+ &dev_attr_cfam_id.attr,
+ &dev_attr_send_term.attr,
+ NULL,
+};
+
+static const struct attribute_group cfam_attr_group = {
+ .attrs = cfam_attr,
+};
+
+static const struct attribute_group *cfam_attr_groups[] = {
+ &cfam_attr_group,
+ NULL,
+};
+
+static char *cfam_devnode(struct device *dev, umode_t *mode,
+ kuid_t *uid, kgid_t *gid)
+{
+ struct fsi_slave *slave = to_fsi_slave(dev);
+
+#ifdef CONFIG_FSI_NEW_DEV_NODE
+ return kasprintf(GFP_KERNEL, "fsi/cfam%d", slave->cdev_idx);
+#else
+ return kasprintf(GFP_KERNEL, "cfam%d", slave->cdev_idx);
+#endif
+}
+
+static const struct device_type cfam_type = {
+ .name = "cfam",
+ .devnode = cfam_devnode,
+ .groups = cfam_attr_groups
+};
+
static char *fsi_cdev_devnode(struct device *dev, umode_t *mode,
kuid_t *uid, kgid_t *gid)
{
@@ -808,7 +963,7 @@ EXPORT_SYMBOL_GPL(fsi_free_minor);
static int fsi_slave_init(struct fsi_master *master, int link, uint8_t id)
{
- uint32_t chip_id;
+ uint32_t cfam_id;
struct fsi_slave *slave;
uint8_t crc;
__be32 data, llmode;
@@ -826,17 +981,17 @@ static int fsi_slave_init(struct fsi_master *master, int link, uint8_t id)
link, id, rc);
return -ENODEV;
}
- chip_id = be32_to_cpu(data);
+ cfam_id = be32_to_cpu(data);
- crc = crc4(0, chip_id, 32);
+ crc = crc4(0, cfam_id, 32);
if (crc) {
- dev_warn(&master->dev, "slave %02x:%02x invalid chip id CRC!\n",
+ dev_warn(&master->dev, "slave %02x:%02x invalid cfam id CRC!\n",
link, id);
return -EIO;
}
dev_dbg(&master->dev, "fsi: found chip %08x at %02x:%02x:%02x\n",
- chip_id, master->idx, link, id);
+ cfam_id, master->idx, link, id);
/* If we're behind a master that doesn't provide a self-running bus
* clock, put the slave into async mode
@@ -859,10 +1014,14 @@ static int fsi_slave_init(struct fsi_master *master, int link, uint8_t id)
if (!slave)
return -ENOMEM;
- slave->master = master;
+ dev_set_name(&slave->dev, "slave@%02x:%02x", link, id);
+ slave->dev.type = &cfam_type;
slave->dev.parent = &master->dev;
slave->dev.of_node = fsi_slave_find_of_node(master, link, id);
slave->dev.release = fsi_slave_release;
+ device_initialize(&slave->dev);
+ slave->cfam_id = cfam_id;
+ slave->master = master;
slave->link = link;
slave->id = id;
slave->size = FSI_SLAVE_SIZE_23b;
@@ -877,6 +1036,21 @@ static int fsi_slave_init(struct fsi_master *master, int link, uint8_t id)
slave->chip_id = prop;
}
+
+ /* Allocate a minor in the FSI space */
+ rc = __fsi_get_new_minor(slave, fsi_dev_cfam, &slave->dev.devt,
+ &slave->cdev_idx);
+ if (rc)
+ goto err_free;
+
+ /* Create chardev for userspace access */
+ cdev_init(&slave->cdev, &cfam_fops);
+ rc = cdev_device_add(&slave->cdev, &slave->dev);
+ if (rc) {
+ dev_err(&slave->dev, "Error %d creating slave device\n", rc);
+ goto err_free;
+ }
+
rc = fsi_slave_set_smode(slave);
if (rc) {
dev_warn(&master->dev,
@@ -890,30 +1064,11 @@ static int fsi_slave_init(struct fsi_master *master, int link, uint8_t id)
slave->t_send_delay,
slave->t_echo_delay);
- dev_set_name(&slave->dev, "slave@%02x:%02x", link, id);
- rc = device_register(&slave->dev);
- if (rc < 0) {
- dev_warn(&master->dev, "failed to create slave device: %d\n",
- rc);
- put_device(&slave->dev);
- return rc;
- }
-
+ /* Legacy raw file -> to be removed */
rc = device_create_bin_file(&slave->dev, &fsi_slave_raw_attr);
if (rc)
dev_warn(&slave->dev, "failed to create raw attr: %d\n", rc);
- rc = device_create_bin_file(&slave->dev, &fsi_slave_term_attr);
- if (rc)
- dev_warn(&slave->dev, "failed to create term attr: %d\n", rc);
-
- rc = device_create_file(&slave->dev, &dev_attr_send_echo_delays);
- if (rc)
- dev_warn(&slave->dev, "failed to create delay attr: %d\n", rc);
-
- rc = device_create_file(&slave->dev, &dev_attr_chip_id);
- if (rc)
- dev_warn(&slave->dev, "failed to create chip id: %d\n", rc);
rc = fsi_slave_scan(slave);
if (rc)
@@ -921,6 +1076,10 @@ static int fsi_slave_init(struct fsi_master *master, int link, uint8_t id)
rc);
return rc;
+
+ err_free:
+ put_device(&slave->dev);
+ return rc;
}
/* FSI master support */
@@ -1029,7 +1188,10 @@ static int fsi_slave_remove_device(struct device *dev, void *arg)
static int fsi_master_remove_slave(struct device *dev, void *arg)
{
+ struct fsi_slave *slave = to_fsi_slave(dev);
+
device_for_each_child(dev, NULL, fsi_slave_remove_device);
+ cdev_device_del(&slave->cdev, &slave->dev);
put_device(dev);
return 0;
}
--
2.17.1
^ permalink raw reply related
* [PATCH 3/5] fsi: scom: Convert to use the new chardev
From: Benjamin Herrenschmidt @ 2018-07-24 5:05 UTC (permalink / raw)
To: linux-aspeed
In-Reply-To: <20180724050519.31920-1-benh@kernel.crashing.org>
This converts FSI scom to use the new fsi-core controlled
chardev allocator and use a real cdev instead of a miscdev.
Signed-off-by: Benjamin Herrenschmidt <benh@kernel.crashing.org>
---
drivers/fsi/fsi-scom.c | 130 +++++++++++++++++++++++++----------------
1 file changed, 80 insertions(+), 50 deletions(-)
diff --git a/drivers/fsi/fsi-scom.c b/drivers/fsi/fsi-scom.c
index 39c74351f1bf..0f303a700f69 100644
--- a/drivers/fsi/fsi-scom.c
+++ b/drivers/fsi/fsi-scom.c
@@ -20,9 +20,8 @@
#include <linux/fs.h>
#include <linux/uaccess.h>
#include <linux/slab.h>
-#include <linux/miscdevice.h>
+#include <linux/cdev.h>
#include <linux/list.h>
-#include <linux/idr.h>
#include <uapi/linux/fsi.h>
@@ -77,18 +76,12 @@
struct scom_device {
struct list_head link;
struct fsi_device *fsi_dev;
- struct miscdevice mdev;
+ struct device dev;
+ struct cdev cdev;
struct mutex lock;
- char name[32];
- int idx;
+ bool dead;
};
-#define to_scom_dev(x) container_of((x), struct scom_device, mdev)
-
-static struct list_head scom_devices;
-
-static DEFINE_IDA(scom_ida);
-
static int __put_scom(struct scom_device *scom_dev, uint64_t value,
uint32_t addr, uint32_t *status)
{
@@ -374,9 +367,7 @@ static int get_scom(struct scom_device *scom, uint64_t *value,
static ssize_t scom_read(struct file *filep, char __user *buf, size_t len,
loff_t *offset)
{
- struct miscdevice *mdev =
- (struct miscdevice *)filep->private_data;
- struct scom_device *scom = to_scom_dev(mdev);
+ struct scom_device *scom = filep->private_data;
struct device *dev = &scom->fsi_dev->dev;
uint64_t val;
int rc;
@@ -385,7 +376,10 @@ static ssize_t scom_read(struct file *filep, char __user *buf, size_t len,
return -EINVAL;
mutex_lock(&scom->lock);
- rc = get_scom(scom, &val, *offset);
+ if (scom->dead)
+ rc = -ENODEV;
+ else
+ rc = get_scom(scom, &val, *offset);
mutex_unlock(&scom->lock);
if (rc) {
dev_dbg(dev, "get_scom fail:%d\n", rc);
@@ -403,8 +397,7 @@ static ssize_t scom_write(struct file *filep, const char __user *buf,
size_t len, loff_t *offset)
{
int rc;
- struct miscdevice *mdev = filep->private_data;
- struct scom_device *scom = to_scom_dev(mdev);
+ struct scom_device *scom = filep->private_data;
struct device *dev = &scom->fsi_dev->dev;
uint64_t val;
@@ -418,7 +411,10 @@ static ssize_t scom_write(struct file *filep, const char __user *buf,
}
mutex_lock(&scom->lock);
- rc = put_scom(scom, val, *offset);
+ if (scom->dead)
+ rc = -ENODEV;
+ else
+ rc = put_scom(scom, val, *offset);
mutex_unlock(&scom->lock);
if (rc) {
dev_dbg(dev, "put_scom failed with:%d\n", rc);
@@ -532,12 +528,15 @@ static int scom_check(struct scom_device *scom, void __user *argp)
static long scom_ioctl(struct file *file, unsigned int cmd, unsigned long arg)
{
- struct miscdevice *mdev = file->private_data;
- struct scom_device *scom = to_scom_dev(mdev);
+ struct scom_device *scom = file->private_data;
void __user *argp = (void __user *)arg;
int rc = -ENOTTY;
mutex_lock(&scom->lock);
+ if (scom->dead) {
+ mutex_unlock(&scom->lock);
+ return -ENODEV;
+ }
switch(cmd) {
case FSI_SCOM_CHECK:
rc = scom_check(scom, argp);
@@ -556,48 +555,88 @@ static long scom_ioctl(struct file *file, unsigned int cmd, unsigned long arg)
return rc;
}
+static int scom_open(struct inode *inode, struct file *file)
+{
+ struct scom_device *scom = container_of(inode->i_cdev, struct scom_device, cdev);
+
+ file->private_data = scom;
+
+ return 0;
+}
+
static const struct file_operations scom_fops = {
.owner = THIS_MODULE,
+ .open = scom_open,
.llseek = scom_llseek,
.read = scom_read,
.write = scom_write,
.unlocked_ioctl = scom_ioctl,
};
+static void scom_free(struct device *dev)
+{
+ struct scom_device *scom = container_of(dev, struct scom_device, dev);
+
+ put_device(&scom->fsi_dev->dev);
+ kfree(scom);
+}
+
static int scom_probe(struct device *dev)
{
struct fsi_device *fsi_dev = to_fsi_dev(dev);
struct scom_device *scom;
+ int rc, didx;
- scom = devm_kzalloc(dev, sizeof(*scom), GFP_KERNEL);
+ scom = kzalloc(sizeof(*scom), GFP_KERNEL);
if (!scom)
return -ENOMEM;
-
+ dev_set_drvdata(dev, scom);
mutex_init(&scom->lock);
- scom->idx = ida_simple_get(&scom_ida, 1, INT_MAX, GFP_KERNEL);
- snprintf(scom->name, sizeof(scom->name), "scom%d", scom->idx);
- scom->fsi_dev = fsi_dev;
- scom->mdev.minor = MISC_DYNAMIC_MINOR;
- scom->mdev.fops = &scom_fops;
- scom->mdev.name = scom->name;
- scom->mdev.parent = dev;
- list_add(&scom->link, &scom_devices);
-
- return misc_register(&scom->mdev);
+
+ /* Grab a reference to the device (parent of our cdev), we'll drop it later */
+ if (!get_device(dev)) {
+ kfree(scom);
+ return -ENODEV;
+ }
+
+ /* Create chardev for userspace access */
+ scom->dev.type = &fsi_cdev_type;
+ scom->dev.parent = dev;
+ scom->dev.release = scom_free;
+ device_initialize(&scom->dev);
+
+ /* Allocate a minor in the FSI space */
+ rc = fsi_get_new_minor(fsi_dev, fsi_dev_scom, &scom->dev.devt, &didx);
+ if (rc)
+ goto err;
+
+ dev_set_name(&scom->dev, "scom%d", didx);
+ cdev_init(&scom->cdev, &scom_fops);
+ rc = cdev_device_add(&scom->cdev, &scom->dev);
+ if (rc) {
+ dev_err(dev, "Error %d creating char device %s\n",
+ rc, dev_name(&scom->dev));
+ goto err_free_minor;
+ }
+
+ return 0;
+ err_free_minor:
+ fsi_free_minor(scom->dev.devt);
+ err:
+ put_device(&scom->dev);
+ return rc;
}
static int scom_remove(struct device *dev)
{
- struct scom_device *scom, *scom_tmp;
- struct fsi_device *fsi_dev = to_fsi_dev(dev);
+ struct scom_device *scom = dev_get_drvdata(dev);
- list_for_each_entry_safe(scom, scom_tmp, &scom_devices, link) {
- if (scom->fsi_dev == fsi_dev) {
- list_del(&scom->link);
- ida_simple_remove(&scom_ida, scom->idx);
- misc_deregister(&scom->mdev);
- }
- }
+ mutex_lock(&scom->lock);
+ scom->dead = true;
+ mutex_unlock(&scom->lock);
+ cdev_device_del(&scom->cdev, &scom->dev);
+ fsi_free_minor(scom->dev.devt);
+ put_device(&scom->dev);
return 0;
}
@@ -622,20 +661,11 @@ static struct fsi_driver scom_drv = {
static int scom_init(void)
{
- INIT_LIST_HEAD(&scom_devices);
return fsi_driver_register(&scom_drv);
}
static void scom_exit(void)
{
- struct list_head *pos;
- struct scom_device *scom;
-
- list_for_each(pos, &scom_devices) {
- scom = list_entry(pos, struct scom_device, link);
- misc_deregister(&scom->mdev);
- devm_kfree(&scom->fsi_dev->dev, scom);
- }
fsi_driver_unregister(&scom_drv);
}
--
2.17.1
^ permalink raw reply related
* [PATCH 2/5] fsi: sbefifo: Convert to use the new chardev
From: Benjamin Herrenschmidt @ 2018-07-24 5:05 UTC (permalink / raw)
To: linux-aspeed
In-Reply-To: <20180724050519.31920-1-benh@kernel.crashing.org>
This converts FSI sbefifo to use the new fsi-core controlled
chardev allocator and use a real cdev instead of a miscdev.
One side effect is to fix the object lifetime by removing
the use of devm_kzalloc() for something that contains kobjects,
and using proper reference counting.
Signed-off-by: Benjamin Herrenschmidt <benh@kernel.crashing.org>
---
drivers/fsi/fsi-sbefifo.c | 84 +++++++++++++++++++++++++--------------
1 file changed, 55 insertions(+), 29 deletions(-)
diff --git a/drivers/fsi/fsi-sbefifo.c b/drivers/fsi/fsi-sbefifo.c
index 33a5d9a43a07..8a185f53c9d8 100644
--- a/drivers/fsi/fsi-sbefifo.c
+++ b/drivers/fsi/fsi-sbefifo.c
@@ -17,9 +17,8 @@
#include <linux/fs.h>
#include <linux/fsi.h>
#include <linux/fsi-sbefifo.h>
-#include <linux/idr.h>
#include <linux/kernel.h>
-#include <linux/miscdevice.h>
+#include <linux/cdev.h>
#include <linux/module.h>
#include <linux/mutex.h>
#include <linux/of.h>
@@ -118,11 +117,11 @@ struct sbefifo {
uint32_t magic;
#define SBEFIFO_MAGIC 0x53424546 /* "SBEF" */
struct fsi_device *fsi_dev;
- struct miscdevice mdev;
+ struct device dev;
+ struct cdev cdev;
struct mutex lock;
- char name[32];
- int idx;
bool broken;
+ bool dead;
bool async_ffdc;
};
@@ -133,9 +132,9 @@ struct sbefifo_user {
size_t pending_len;
};
-static DEFINE_IDA(sbefifo_ida);
static DEFINE_MUTEX(sbefifo_ffdc_mutex);
+
static void __sbefifo_dump_ffdc(struct device *dev, const __be32 *ffdc,
size_t ffdc_sz, bool internal)
{
@@ -667,6 +666,9 @@ static int __sbefifo_submit(struct sbefifo *sbefifo,
struct device *dev = &sbefifo->fsi_dev->dev;
int rc;
+ if (sbefifo->dead)
+ return -ENODEV;
+
if (cmd_len < 2 || be32_to_cpu(command[0]) != cmd_len) {
dev_vdbg(dev, "Invalid command len %zd (header: %d)\n",
cmd_len, be32_to_cpu(command[0]));
@@ -751,8 +753,7 @@ EXPORT_SYMBOL_GPL(sbefifo_submit);
*/
static int sbefifo_user_open(struct inode *inode, struct file *file)
{
- struct sbefifo *sbefifo = container_of(file->private_data,
- struct sbefifo, mdev);
+ struct sbefifo *sbefifo = container_of(inode->i_cdev, struct sbefifo, cdev);
struct sbefifo_user *user;
user = kzalloc(sizeof(struct sbefifo_user), GFP_KERNEL);
@@ -889,6 +890,14 @@ static const struct file_operations sbefifo_fops = {
.release = sbefifo_user_release,
};
+static void sbefifo_free(struct device *dev)
+{
+ struct sbefifo *sbefifo = container_of(dev, struct sbefifo, dev);
+
+ put_device(&sbefifo->fsi_dev->dev);
+ kfree(sbefifo);
+}
+
/*
* Probe/remove
*/
@@ -900,15 +909,23 @@ static int sbefifo_probe(struct device *dev)
struct device_node *np;
struct platform_device *child;
char child_name[32];
- int rc, child_idx = 0;
+ int rc, didx, child_idx = 0;
dev_dbg(dev, "Found sbefifo device\n");
- sbefifo = devm_kzalloc(dev, sizeof(*sbefifo), GFP_KERNEL);
+ sbefifo = kzalloc(sizeof(*sbefifo), GFP_KERNEL);
if (!sbefifo)
return -ENOMEM;
+
+ /* Grab a reference to the device (parent of our cdev), we'll drop it later */
+ if (!get_device(dev)) {
+ return -ENODEV;
+ kfree(sbefifo);
+ }
+
sbefifo->magic = SBEFIFO_MAGIC;
sbefifo->fsi_dev = fsi_dev;
+ dev_set_drvdata(dev, sbefifo);
mutex_init(&sbefifo->lock);
/*
@@ -919,28 +936,30 @@ static int sbefifo_probe(struct device *dev)
if (rc && rc != -ESHUTDOWN)
dev_err(dev, "Initial HW cleanup failed, will retry later\n");
- sbefifo->idx = ida_simple_get(&sbefifo_ida, 1, INT_MAX, GFP_KERNEL);
- snprintf(sbefifo->name, sizeof(sbefifo->name), "sbefifo%d",
- sbefifo->idx);
+ /* Create chardev for userspace access */
+ sbefifo->dev.type = &fsi_cdev_type;
+ sbefifo->dev.parent = dev;
+ sbefifo->dev.release = sbefifo_free;
+ device_initialize(&sbefifo->dev);
- dev_set_drvdata(dev, sbefifo);
+ /* Allocate a minor in the FSI space */
+ rc = fsi_get_new_minor(fsi_dev, fsi_dev_sbefifo, &sbefifo->dev.devt, &didx);
+ if (rc)
+ goto err;
- /* Create misc chardev for userspace access */
- sbefifo->mdev.minor = MISC_DYNAMIC_MINOR;
- sbefifo->mdev.fops = &sbefifo_fops;
- sbefifo->mdev.name = sbefifo->name;
- sbefifo->mdev.parent = dev;
- rc = misc_register(&sbefifo->mdev);
+ dev_set_name(&sbefifo->dev, "sbefifo%d", didx);
+ cdev_init(&sbefifo->cdev, &sbefifo_fops);
+ rc = cdev_device_add(&sbefifo->cdev, &sbefifo->dev);
if (rc) {
- dev_err(dev, "Failed to register miscdevice: %d\n", rc);
- ida_simple_remove(&sbefifo_ida, sbefifo->idx);
- return rc;
+ dev_err(dev, "Error %d creating char device %s\n",
+ rc, dev_name(&sbefifo->dev));
+ goto err_free_minor;
}
/* Create platform devs for dts child nodes (occ, etc) */
for_each_available_child_of_node(dev->of_node, np) {
snprintf(child_name, sizeof(child_name), "%s-dev%d",
- sbefifo->name, child_idx++);
+ dev_name(&sbefifo->dev), child_idx++);
child = of_platform_device_create(np, child_name, dev);
if (!child)
dev_warn(dev, "failed to create child %s dev\n",
@@ -948,6 +967,11 @@ static int sbefifo_probe(struct device *dev)
}
return 0;
+ err_free_minor:
+ fsi_free_minor(sbefifo->dev.devt);
+ err:
+ put_device(&sbefifo->dev);
+ return rc;
}
static int sbefifo_unregister_child(struct device *dev, void *data)
@@ -967,10 +991,14 @@ static int sbefifo_remove(struct device *dev)
dev_dbg(dev, "Removing sbefifo device...\n");
- misc_deregister(&sbefifo->mdev);
- device_for_each_child(dev, NULL, sbefifo_unregister_child);
+ mutex_lock(&sbefifo->lock);
+ sbefifo->dead = true;
+ mutex_unlock(&sbefifo->lock);
- ida_simple_remove(&sbefifo_ida, sbefifo->idx);
+ cdev_device_del(&sbefifo->cdev, &sbefifo->dev);
+ fsi_free_minor(sbefifo->dev.devt);
+ device_for_each_child(dev, NULL, sbefifo_unregister_child);
+ put_device(&sbefifo->dev);
return 0;
}
@@ -1001,8 +1029,6 @@ static int sbefifo_init(void)
static void sbefifo_exit(void)
{
fsi_driver_unregister(&sbefifo_drv);
-
- ida_destroy(&sbefifo_ida);
}
module_init(sbefifo_init);
--
2.17.1
^ permalink raw reply related
* [PATCH 1/5] fsi: Add new central chardev support
From: Benjamin Herrenschmidt @ 2018-07-24 5:05 UTC (permalink / raw)
To: linux-aspeed
In-Reply-To: <20180724050519.31920-1-benh@kernel.crashing.org>
The various FSI devices (sbefifo, occ, scom, more to come)
currently use misc devices.
This is problematic as the minor device space for misc is
limited and there can be a lot of them. Also it limits our
ability to move them to a dedicated /dev/fsi directory or
to be smart about device naming and numbering.
It also means we have IDAs on every single of these drivers
This creates a common fsi "device_type" for the optional
/dev/fsi grouping and a dev_t allocator for all FSI devices.
"Legacy" devices get to use a backward compatible numbering
scheme (as long as chip id <16 and there's only one copy
of a given unit type per chip).
A single major number and a single IDA are shared for all
FSI devices.
This doesn't convert the FSI device drivers to use the new
scheme yet, they will be converted individually.
Signed-off-by: Benjamin Herrenschmidt <benh@kernel.crashing.org>
---
drivers/fsi/Kconfig | 15 +++++++
drivers/fsi/fsi-core.c | 95 +++++++++++++++++++++++++++++++++++++++++-
include/linux/fsi.h | 12 +++++-
3 files changed, 119 insertions(+), 3 deletions(-)
diff --git a/drivers/fsi/Kconfig b/drivers/fsi/Kconfig
index 8d82b1e60514..af3a20dd5aa4 100644
--- a/drivers/fsi/Kconfig
+++ b/drivers/fsi/Kconfig
@@ -12,6 +12,21 @@ menuconfig FSI
if FSI
+config FSI_NEW_DEV_NODE
+ bool "Create '/dev/fsi' directory for char devices"
+ default n
+ ---help---
+ This option causes char devices created for FSI devices to be
+ located under a common /dev/fsi/ directory. Set to N unless your
+ userspace has been updated to handle the new location.
+
+ Additionally, it also causes the char device names to be offset
+ by one so that chip 0 will have /dev/scom1 and chip1 /dev/scom2
+ to match old userspace expectations.
+
+ New userspace will use udev rules to generate predictable access
+ symlinks in /dev/fsi/by-path when this option is enabled.
+
config FSI_MASTER_GPIO
tristate "GPIO-based FSI master"
depends on GPIOLIB
diff --git a/drivers/fsi/fsi-core.c b/drivers/fsi/fsi-core.c
index eab6c5c4990e..faa1760a5a40 100644
--- a/drivers/fsi/fsi-core.c
+++ b/drivers/fsi/fsi-core.c
@@ -92,6 +92,13 @@ struct fsi_slave {
static const int slave_retries = 2;
static int discard_errors;
+static dev_t fsi_base_dev;
+static DEFINE_IDA(fsi_minor_ida);
+#define FSI_CHAR_MAX_DEVICES 0x1000
+
+/* Legacy /dev numbering: 4 devices per chip, 16 chips */
+#define FSI_CHAR_LEGACY_TOP 64
+
static int fsi_master_read(struct fsi_master *master, int link,
uint8_t slave_id, uint32_t addr, void *val, size_t size);
static int fsi_master_write(struct fsi_master *master, int link,
@@ -627,6 +634,7 @@ static void fsi_slave_release(struct device *dev)
{
struct fsi_slave *slave = to_fsi_slave(dev);
+ fsi_free_minor(slave->dev.devt);
of_node_put(dev->of_node);
kfree(slave);
}
@@ -729,6 +737,75 @@ static ssize_t chip_id_show(struct device *dev,
static DEVICE_ATTR_RO(chip_id);
+static char *fsi_cdev_devnode(struct device *dev, umode_t *mode,
+ kuid_t *uid, kgid_t *gid)
+{
+#ifdef CONFIG_FSI_NEW_DEV_NODE
+ return kasprintf(GFP_KERNEL, "fsi/%s", dev_name(dev));
+#else
+ return kasprintf(GFP_KERNEL, "%s", dev_name(dev));
+#endif
+}
+
+const struct device_type fsi_cdev_type = {
+ .name = "fsi-cdev",
+ .devnode = fsi_cdev_devnode,
+};
+EXPORT_SYMBOL_GPL(fsi_cdev_type);
+
+/* Backward compatible /dev/ numbering in "old style" mode */
+static int fsi_adjust_index(int index)
+{
+#ifdef CONFIG_FSI_NEW_DEV_NODE
+ return index;
+#else
+ return index + 1;
+#endif
+}
+
+static int __fsi_get_new_minor(struct fsi_slave *slave, enum fsi_dev_type type,
+ dev_t *out_dev, int *out_index)
+{
+ int cid = slave->chip_id;
+ int id;
+
+ /* Check if we qualify for legacy numbering */
+ if (cid >= 0 && cid < 16 && type < 4) {
+ /* Try reserving the legacy number */
+ id = (cid << 4) | type;
+ id = ida_simple_get(&fsi_minor_ida, id, id + 1, GFP_KERNEL);
+ if (id >= 0) {
+ *out_index = fsi_adjust_index(cid);
+ *out_dev = fsi_base_dev + id;
+ return 0;
+ }
+ /* Other failure */
+ if (id != -ENOSPC)
+ return id;
+ /* Fallback to non-legacy allocation */
+ }
+ id = ida_simple_get(&fsi_minor_ida, FSI_CHAR_LEGACY_TOP,
+ FSI_CHAR_MAX_DEVICES, GFP_KERNEL);
+ if (id < 0)
+ return id;
+ *out_index = fsi_adjust_index(id);
+ *out_dev = fsi_base_dev + id;
+ return 0;
+}
+
+int fsi_get_new_minor(struct fsi_device *fdev, enum fsi_dev_type type,
+ dev_t *out_dev, int *out_index)
+{
+ return __fsi_get_new_minor(fdev->slave, type, out_dev, out_index);
+}
+EXPORT_SYMBOL_GPL(fsi_get_new_minor);
+
+void fsi_free_minor(dev_t dev)
+{
+ ida_simple_remove(&fsi_minor_ida, MINOR(dev));
+}
+EXPORT_SYMBOL_GPL(fsi_free_minor);
+
static int fsi_slave_init(struct fsi_master *master, int link, uint8_t id)
{
uint32_t chip_id;
@@ -953,7 +1030,7 @@ static int fsi_slave_remove_device(struct device *dev, void *arg)
static int fsi_master_remove_slave(struct device *dev, void *arg)
{
device_for_each_child(dev, NULL, fsi_slave_remove_device);
- device_unregister(dev);
+ put_device(dev);
return 0;
}
@@ -1091,13 +1168,27 @@ EXPORT_SYMBOL_GPL(fsi_bus_type);
static int __init fsi_init(void)
{
- return bus_register(&fsi_bus_type);
+ int rc;
+
+ rc = alloc_chrdev_region(&fsi_base_dev, 0, FSI_CHAR_MAX_DEVICES, "fsi");
+ if (rc)
+ return rc;
+ rc = bus_register(&fsi_bus_type);
+ if (rc)
+ goto fail_bus;
+ return 0;
+
+ fail_bus:
+ unregister_chrdev_region(fsi_base_dev, FSI_CHAR_MAX_DEVICES);
+ return rc;
}
postcore_initcall(fsi_init);
static void fsi_exit(void)
{
bus_unregister(&fsi_bus_type);
+ unregister_chrdev_region(fsi_base_dev, FSI_CHAR_MAX_DEVICES);
+ ida_destroy(&fsi_minor_ida);
}
module_exit(fsi_exit);
module_param(discard_errors, int, 0664);
diff --git a/include/linux/fsi.h b/include/linux/fsi.h
index 141fd38d061f..ec3be0d5b786 100644
--- a/include/linux/fsi.h
+++ b/include/linux/fsi.h
@@ -76,8 +76,18 @@ extern int fsi_slave_read(struct fsi_slave *slave, uint32_t addr,
extern int fsi_slave_write(struct fsi_slave *slave, uint32_t addr,
const void *val, size_t size);
+extern struct bus_type fsi_bus_type;
+extern const struct device_type fsi_cdev_type;
+enum fsi_dev_type {
+ fsi_dev_cfam,
+ fsi_dev_sbefifo,
+ fsi_dev_scom,
+ fsi_dev_occ
+};
-extern struct bus_type fsi_bus_type;
+extern int fsi_get_new_minor(struct fsi_device *fdev, enum fsi_dev_type type,
+ dev_t *out_dev, int *out_index);
+extern void fsi_free_minor(dev_t dev);
#endif /* LINUX_FSI_H */
--
2.17.1
^ permalink raw reply related
* [PATCH 0/5] fsi: Convert misc devs to proper chardevs and more
From: Benjamin Herrenschmidt @ 2018-07-24 5:05 UTC (permalink / raw)
To: linux-aspeed
This converts the various FSI devices from misc dev to chardev,
as there can potentially be too much of them for misc devs limited
minors, and because there are some lifetime issues with the current
support.
This provide a common infrastructure to allocate an FSI major and
distribute minors in a way that keeps it compatible with existing
userspace. A new representation grouping FSI devices under a
/dev/fsi directory is optinally provided, which will work in
conjunction with new udev scripts aimed at providing fixed ID
based symlinks.
A side effect of those conversions is to fix some object lifetime
issues caused by a mixup between devm_kzalloc and proper object
lifetime.
This series also adds a /dev{/fsi}/cfamN chardev for raw CFAM
access that will superseed the existing "raw" sysfs file.
Finally there's also a locking fix to avoid horrible mixups if
the "rescan" file is poked while a rescan is already in progress.
^ permalink raw reply
* [PATCH 3/3] ARM: config: multi_v5: Enable ASPEED drivers
From: Andrew Jeffery @ 2018-07-24 5:05 UTC (permalink / raw)
To: linux-aspeed
In-Reply-To: <CACPK8Xd0ViuVAHgD4Nem5e_th0pf=4hmC=3d0XbLRGGzcDTMDg@mail.gmail.com>
On Tue, 24 Jul 2018, at 14:29, Joel Stanley wrote:
> On 24 July 2018 at 14:26, Andrew Jeffery <andrew@aj.id.au> wrote:
> >
> >
> > On Wed, 18 Jul 2018, at 23:23, Joel Stanley wrote:
> >> This enables the devices used in the AST2400 family of BMC SoCs:
> >>
> >> - VUART
> >> - SPI NOR
> >> - LPC controller
> >> - LPC snoop (port 80)
> >> - Ethernet
> >> - GPIO
> >> - ADC
> >> - I2C
> >> - Random number generator
> >> - IPMI KCS
> >> - IPMI BT
> >> - Fan/Tach
> >>
> >> Signed-off-by: Joel Stanley <joel@jms.id.au>
> >> ---
> >> arch/arm/configs/multi_v5_defconfig | 14 +++++++++++++-
> >> 1 file changed, 13 insertions(+), 1 deletion(-)
> >>
> >> diff --git a/arch/arm/configs/multi_v5_defconfig b/arch/arm/configs/
> >> multi_v5_defconfig
> >> index b647e249908e..318b76fa26d1 100644
> >> --- a/arch/arm/configs/multi_v5_defconfig
> >> +++ b/arch/arm/configs/multi_v5_defconfig
> >> @@ -90,10 +90,14 @@ CONFIG_MTD_PHYSMAP=y
> >> CONFIG_MTD_NAND=y
> >> CONFIG_MTD_NAND_ATMEL=y
> >> CONFIG_MTD_NAND_ORION=y
> >> +CONFIG_MTD_SPI_NOR=y
> >> +CONFIG_SPI_ASPEED_SMC=y
> >> CONFIG_MTD_UBI=y
> >> CONFIG_BLK_DEV_LOOP=y
> >> CONFIG_ATMEL_TCLIB=y
> >> CONFIG_ATMEL_SSC=m
> >> +CONFIG_ASPEED_LPC_CTRL=m
> >> +CONFIG_ASPEED_LPC_SNOOP=m
> >
> > Any reason that you switch between module and built-in throughout? It's not clear from the commit message.
>
> I tried to follow what the existing config did. Some things are
> modules, but some subsystems aren't.
>
> I figured there's no harm in getting some build coverage of our
> drivers being modules.
Righto.
Acked-by: Andrew Jeffery <andrew@aj.id.au>
>
> Cheers,
>
> Joel
^ permalink raw reply
* [PATCH 3/3] ARM: config: multi_v5: Enable ASPEED drivers
From: Joel Stanley @ 2018-07-24 4:59 UTC (permalink / raw)
To: linux-aspeed
In-Reply-To: <1532408201.2611907.1450747808.5F518C30@webmail.messagingengine.com>
On 24 July 2018 at 14:26, Andrew Jeffery <andrew@aj.id.au> wrote:
>
>
> On Wed, 18 Jul 2018, at 23:23, Joel Stanley wrote:
>> This enables the devices used in the AST2400 family of BMC SoCs:
>>
>> - VUART
>> - SPI NOR
>> - LPC controller
>> - LPC snoop (port 80)
>> - Ethernet
>> - GPIO
>> - ADC
>> - I2C
>> - Random number generator
>> - IPMI KCS
>> - IPMI BT
>> - Fan/Tach
>>
>> Signed-off-by: Joel Stanley <joel@jms.id.au>
>> ---
>> arch/arm/configs/multi_v5_defconfig | 14 +++++++++++++-
>> 1 file changed, 13 insertions(+), 1 deletion(-)
>>
>> diff --git a/arch/arm/configs/multi_v5_defconfig b/arch/arm/configs/
>> multi_v5_defconfig
>> index b647e249908e..318b76fa26d1 100644
>> --- a/arch/arm/configs/multi_v5_defconfig
>> +++ b/arch/arm/configs/multi_v5_defconfig
>> @@ -90,10 +90,14 @@ CONFIG_MTD_PHYSMAP=y
>> CONFIG_MTD_NAND=y
>> CONFIG_MTD_NAND_ATMEL=y
>> CONFIG_MTD_NAND_ORION=y
>> +CONFIG_MTD_SPI_NOR=y
>> +CONFIG_SPI_ASPEED_SMC=y
>> CONFIG_MTD_UBI=y
>> CONFIG_BLK_DEV_LOOP=y
>> CONFIG_ATMEL_TCLIB=y
>> CONFIG_ATMEL_SSC=m
>> +CONFIG_ASPEED_LPC_CTRL=m
>> +CONFIG_ASPEED_LPC_SNOOP=m
>
> Any reason that you switch between module and built-in throughout? It's not clear from the commit message.
I tried to follow what the existing config did. Some things are
modules, but some subsystems aren't.
I figured there's no harm in getting some build coverage of our
drivers being modules.
Cheers,
Joel
^ permalink raw reply
* [PATCH 3/3] ARM: config: multi_v5: Enable ASPEED drivers
From: Andrew Jeffery @ 2018-07-24 4:56 UTC (permalink / raw)
To: linux-aspeed
In-Reply-To: <20180718135302.4927-4-joel@jms.id.au>
On Wed, 18 Jul 2018, at 23:23, Joel Stanley wrote:
> This enables the devices used in the AST2400 family of BMC SoCs:
>
> - VUART
> - SPI NOR
> - LPC controller
> - LPC snoop (port 80)
> - Ethernet
> - GPIO
> - ADC
> - I2C
> - Random number generator
> - IPMI KCS
> - IPMI BT
> - Fan/Tach
>
> Signed-off-by: Joel Stanley <joel@jms.id.au>
> ---
> arch/arm/configs/multi_v5_defconfig | 14 +++++++++++++-
> 1 file changed, 13 insertions(+), 1 deletion(-)
>
> diff --git a/arch/arm/configs/multi_v5_defconfig b/arch/arm/configs/
> multi_v5_defconfig
> index b647e249908e..318b76fa26d1 100644
> --- a/arch/arm/configs/multi_v5_defconfig
> +++ b/arch/arm/configs/multi_v5_defconfig
> @@ -90,10 +90,14 @@ CONFIG_MTD_PHYSMAP=y
> CONFIG_MTD_NAND=y
> CONFIG_MTD_NAND_ATMEL=y
> CONFIG_MTD_NAND_ORION=y
> +CONFIG_MTD_SPI_NOR=y
> +CONFIG_SPI_ASPEED_SMC=y
> CONFIG_MTD_UBI=y
> CONFIG_BLK_DEV_LOOP=y
> CONFIG_ATMEL_TCLIB=y
> CONFIG_ATMEL_SSC=m
> +CONFIG_ASPEED_LPC_CTRL=m
> +CONFIG_ASPEED_LPC_SNOOP=m
Any reason that you switch between module and built-in throughout? It's not clear from the commit message.
> CONFIG_EEPROM_AT24=y
> # CONFIG_SCSI_PROC_FS is not set
> CONFIG_BLK_DEV_SD=y
> @@ -107,6 +111,7 @@ CONFIG_NET_DSA_MV88E6060=y
> CONFIG_NET_DSA_MV88E6XXX=y
> CONFIG_MACB=y
> CONFIG_DM9000=y
> +CONFIG_FTGMAC100=m
> CONFIG_MV643XX_ETH=y
> CONFIG_R8169=y
> CONFIG_DAVICOM_PHY=y
> @@ -128,16 +133,20 @@ CONFIG_SERIAL_8250_NR_UARTS=6
> CONFIG_SERIAL_8250_RUNTIME_UARTS=6
> CONFIG_SERIAL_8250_EXTENDED=y
> CONFIG_SERIAL_8250_MANY_PORTS=y
> +CONFIG_SERIAL_8250_ASPEED_VUART=m
> CONFIG_SERIAL_OF_PLATFORM=y
> CONFIG_SERIAL_ATMEL=y
> CONFIG_SERIAL_ATMEL_CONSOLE=y
> CONFIG_SERIAL_ATMEL_TTYAT=y
> CONFIG_SERIAL_IMX=y
> CONFIG_SERIAL_IMX_CONSOLE=y
> +CONFIG_ASPEED_KCS_IPMI_BMC=m
> +CONFIG_ASPEED_BT_IPMI_BMC=m
> CONFIG_HW_RANDOM=y
> -CONFIG_I2C=y
> +CONFIG_HW_RANDOM_TIMERIOMEM=m
> # CONFIG_I2C_COMPAT is not set
> CONFIG_I2C_CHARDEV=y
> +CONFIG_I2C_ASPEED=m
> CONFIG_I2C_AT91=y
> CONFIG_I2C_IMX=y
> CONFIG_I2C_MV64XXX=y
> @@ -146,10 +155,12 @@ CONFIG_SPI=y
> CONFIG_SPI_ATMEL=y
> CONFIG_SPI_IMX=y
> CONFIG_SPI_ORION=y
> +CONFIG_GPIO_ASPEED=m
> CONFIG_POWER_RESET=y
> CONFIG_POWER_RESET_GPIO=y
> CONFIG_POWER_RESET_QNAP=y
> CONFIG_SENSORS_ADT7475=y
> +CONFIG_SENSORS_ASPEED=y
> CONFIG_SENSORS_G762=y
> CONFIG_SENSORS_LM63=y
> CONFIG_SENSORS_LM75=y
> @@ -240,6 +251,7 @@ CONFIG_MV_XOR=y
> CONFIG_STAGING=y
> CONFIG_FB_XGI=y
> CONFIG_IIO=m
> +CONFIG_ASPEED_ADC=m
> CONFIG_AT91_ADC=m
> CONFIG_PWM=y
> CONFIG_PWM_ATMEL=m
> --
> 2.17.1
>
^ permalink raw reply
* [PATCH 2/3] ARM: config: multi_v5: Refresh configuration
From: Andrew Jeffery @ 2018-07-24 4:52 UTC (permalink / raw)
To: linux-aspeed
In-Reply-To: <20180718135302.4927-3-joel@jms.id.au>
On Wed, 18 Jul 2018, at 23:23, Joel Stanley wrote:
> This is the result of a make mutli_v5_defconfig && make savedefconfig.
Typo: s/mutli/multi/
Otherwise,
Acked-by: Andrew Jeffery <andrew@aj.id.au>
>
> Signed-off-by: Joel Stanley <joel@jms.id.au>
> ---
> arch/arm/configs/multi_v5_defconfig | 16 +++++-----------
> 1 file changed, 5 insertions(+), 11 deletions(-)
>
> diff --git a/arch/arm/configs/multi_v5_defconfig b/arch/arm/configs/
> multi_v5_defconfig
> index 7c41bee28463..b647e249908e 100644
> --- a/arch/arm/configs/multi_v5_defconfig
> +++ b/arch/arm/configs/multi_v5_defconfig
> @@ -1,5 +1,4 @@
> CONFIG_SYSVIPC=y
> -CONFIG_FHANDLE=y
> CONFIG_NO_HZ=y
> CONFIG_HIGH_RES_TIMERS=y
> CONFIG_LOG_BUF_SHIFT=19
> @@ -11,12 +10,10 @@ CONFIG_KPROBES=y
> CONFIG_MODULES=y
> CONFIG_MODULE_UNLOAD=y
> # CONFIG_ARCH_MULTI_V7 is not set
> -CONFIG_ARCH_MVEBU=y
> -CONFIG_MACH_KIRKWOOD=y
> -CONFIG_ARCH_AT91=y
> -CONFIG_SOC_AT91SAM9=y
> CONFIG_ARCH_ASPEED=y
> CONFIG_MACH_ASPEED_G4=y
> +CONFIG_ARCH_AT91=y
> +CONFIG_SOC_AT91SAM9=y
> CONFIG_ARCH_MXC=y
> CONFIG_MACH_MX21ADS=y
> CONFIG_MACH_MX27ADS=y
> @@ -25,6 +22,8 @@ CONFIG_MACH_IMX27_VISSTRIM_M10=y
> CONFIG_MACH_PCA100=y
> CONFIG_MACH_IMX27_DT=y
> CONFIG_SOC_IMX25=y
> +CONFIG_ARCH_MVEBU=y
> +CONFIG_MACH_KIRKWOOD=y
> CONFIG_ARCH_ORION5X=y
> CONFIG_MACH_DB88F5281=y
> CONFIG_MACH_RD88F5182=y
> @@ -34,7 +33,6 @@ CONFIG_MACH_DNS323=y
> CONFIG_MACH_TS209=y
> CONFIG_MACH_TERASTATION_PRO2=y
> CONFIG_MACH_LINKSTATION_PRO=y
> -CONFIG_MACH_LINKSTATION_LSCHL=y
> CONFIG_MACH_LINKSTATION_MINI=y
> CONFIG_MACH_LINKSTATION_LS_HGL=y
> CONFIG_MACH_TS409=y
> @@ -71,7 +69,6 @@ CONFIG_IP_PNP=y
> CONFIG_IP_PNP_DHCP=y
> CONFIG_IP_PNP_BOOTP=y
> CONFIG_NET_DSA=y
> -CONFIG_NET_SWITCHDEV=y
> CONFIG_NET_PKTGEN=m
> CONFIG_CFG80211=y
> CONFIG_MAC80211=y
> @@ -112,8 +109,8 @@ CONFIG_MACB=y
> CONFIG_DM9000=y
> CONFIG_MV643XX_ETH=y
> CONFIG_R8169=y
> -CONFIG_MARVELL_PHY=y
> CONFIG_DAVICOM_PHY=y
> +CONFIG_MARVELL_PHY=y
> CONFIG_MICREL_PHY=y
> CONFIG_LIBERTAS=y
> CONFIG_LIBERTAS_SDIO=y
> @@ -125,7 +122,6 @@ CONFIG_KEYBOARD_GPIO=y
> CONFIG_INPUT_TOUCHSCREEN=y
> CONFIG_TOUCHSCREEN_ADS7846=m
> CONFIG_LEGACY_PTY_COUNT=16
> -# CONFIG_DEVKMEM is not set
> CONFIG_SERIAL_8250=y
> CONFIG_SERIAL_8250_CONSOLE=y
> CONFIG_SERIAL_8250_NR_UARTS=6
> @@ -172,11 +168,9 @@ CONFIG_MEDIA_CAMERA_SUPPORT=y
> CONFIG_V4L_PLATFORM_DRIVERS=y
> CONFIG_SOC_CAMERA=y
> CONFIG_VIDEO_ATMEL_ISI=m
> -CONFIG_SOC_CAMERA_OV2640=m
> CONFIG_DRM=y
> CONFIG_DRM_ATMEL_HLCDC=m
> CONFIG_DRM_PANEL_SIMPLE=y
> -CONFIG_FB=y
> CONFIG_FB_IMX=y
> CONFIG_FB_ATMEL=y
> CONFIG_BACKLIGHT_ATMEL_LCDC=y
> --
> 2.17.1
>
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox