Re: [PATCH] HPE BMC GXP SUPPORT
On Fri, 2022-02-04 at 12:31 +, Russell King (Oracle) wrote:
> On Fri, Feb 04, 2022 at 04:18:24AM -0800, Joe Perches wrote:
> > On Fri, 2022-02-04 at 12:05 +, Russell King (Oracle) wrote:
> > > On Wed, Feb 02, 2022 at 10:52:50AM -0600, nick.hawk...@hpe.com wrote:
> > > > + if (readb_relaxed(timer->control) & MASK_TCS_TC) {
> > > > + writeb_relaxed(MASK_TCS_TC, timer->control);
> > > > +
> > > > + event_handler = READ_ONCE(timer->evt.event_handler);
> > > > + if (event_handler)
> > > > + event_handler(>evt);
> > > > + return IRQ_HANDLED;
> > > > + } else {
> > > > + return IRQ_NONE;
> > > > + }
> > > > +}
> >
> > It's also less indented code and perhaps clearer to reverse the test
> >
> > if (!readb_relaxed(timer->control) & MASK_TCS_TC)
>
> This will need to be:
>
> if (!(readb_relaxed(timer->control) & MASK_TCS_TC))
right, thanks.
Re: [PATCH] HPE BMC GXP SUPPORT
> > +#include > > +#include > > +#include > > +#include > > +#include > > +#include > > +#include > > It's normal to list all linux/ includes before asm/ includes. Please > rearrange. Hi Nick Since you are new to the kernel, please let me point out, you should consider Russell comments for all your code, not just this one file. Many of the comments are generic to code anywhere in the kernel. So it would be good to fix the same issues in the rest of your code base before submitting them. I would also suggest that when you start submitting drivers, submit just one or two to start with. You will learn a lot from the feedback you get, and you can apply what you have learnt to the rest of your code before you post them for review. I would also suggest you spend 30 minutes a day just reading comments other patches receive. You can also learn a lot that way, see if the comments apply to your own code. You will also learn about processes this way, which can be just as challenging to get right as code. Andrew
Re: [PATCH] HPE BMC GXP SUPPORT
On Fri, Feb 04, 2022 at 04:18:24AM -0800, Joe Perches wrote:
> On Fri, 2022-02-04 at 12:05 +, Russell King (Oracle) wrote:
> > On Wed, Feb 02, 2022 at 10:52:50AM -0600, nick.hawk...@hpe.com wrote:
> > > + if (readb_relaxed(timer->control) & MASK_TCS_TC) {
> > > + writeb_relaxed(MASK_TCS_TC, timer->control);
> > > +
> > > + event_handler = READ_ONCE(timer->evt.event_handler);
> > > + if (event_handler)
> > > + event_handler(>evt);
> > > + return IRQ_HANDLED;
> > > + } else {
> > > + return IRQ_NONE;
> > > + }
> > > +}
>
> It's also less indented code and perhaps clearer to reverse the test
>
> if (!readb_relaxed(timer->control) & MASK_TCS_TC)
This will need to be:
if (!(readb_relaxed(timer->control) & MASK_TCS_TC))
> return IRQ_NONE;
>
> writeb_relaxed(MASK_TCS_TC, timer->control);
>
> event_handler = READ_ONCE(timer->evt.event_handler);
> if (event_handler)
> event_handler(>evt);
>
> return IRQ_HANDLED;
>
>
>
--
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 40Mbps down 10Mbps up. Decent connectivity at last!
Re: [PATCH] HPE BMC GXP SUPPORT
On Fri, 2022-02-04 at 12:05 +, Russell King (Oracle) wrote:
> On Wed, Feb 02, 2022 at 10:52:50AM -0600, nick.hawk...@hpe.com wrote:
[]
> > diff --git a/drivers/clocksource/Makefile b/drivers/clocksource/Makefile
[]
> > +static irqreturn_t gxp_time_interrupt(int irq, void *dev_id)
> > +{
> > + struct gxp_timer *timer = dev_id;
> > + void (*event_handler)(struct clock_event_device *timer);
> > +
> > +
>
> One too many blank lines.
>
> > + if (readb_relaxed(timer->control) & MASK_TCS_TC) {
> > + writeb_relaxed(MASK_TCS_TC, timer->control);
> > +
> > + event_handler = READ_ONCE(timer->evt.event_handler);
> > + if (event_handler)
> > + event_handler(>evt);
> > + return IRQ_HANDLED;
> > + } else {
> > + return IRQ_NONE;
> > + }
> > +}
It's also less indented code and perhaps clearer to reverse the test
if (!readb_relaxed(timer->control) & MASK_TCS_TC)
return IRQ_NONE;
writeb_relaxed(MASK_TCS_TC, timer->control);
event_handler = READ_ONCE(timer->evt.event_handler);
if (event_handler)
event_handler(>evt);
return IRQ_HANDLED;
Re: [PATCH] HPE BMC GXP SUPPORT
Hi,
On Wed, Feb 02, 2022 at 10:52:50AM -0600, nick.hawk...@hpe.com wrote:
> diff --git a/arch/arm/mach-hpe/Makefile b/arch/arm/mach-hpe/Makefile
> new file mode 100644
> index ..8b0a91234df4
> --- /dev/null
> +++ b/arch/arm/mach-hpe/Makefile
> @@ -0,0 +1 @@
> +obj-$(CONFIG_ARCH_HPE_GXP) += gxp.o
> diff --git a/arch/arm/mach-hpe/gxp.c b/arch/arm/mach-hpe/gxp.c
> new file mode 100644
> index ..a37838247948
> --- /dev/null
> +++ b/arch/arm/mach-hpe/gxp.c
> @@ -0,0 +1,62 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/* Copyright (C) 2022 Hewlett-Packard Enterprise Development Company, L.P.
> + *
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License version 2 as
> + * published by the Free Software Foundation.
> + */
> +
> +
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
It's normal to list all linux/ includes before asm/ includes. Please
rearrange.
> +
> +#define IOP_REGS_PHYS_BASE 0xc000
> +#define IOP_REGS_VIRT_BASE 0xf000
> +#define IOP_REGS_SIZE (240*SZ_1M)
> +
> +#define IOP_EHCI_USBCMD 0x0efe0010
> +
> +static struct map_desc gxp_io_desc[] __initdata = {
> + {
> + .virtual= (unsigned long)IOP_REGS_VIRT_BASE,
> + .pfn= __phys_to_pfn(IOP_REGS_PHYS_BASE),
> + .length = IOP_REGS_SIZE,
> + .type = MT_DEVICE,
If you keep this, please indent the above four lines by one more tab.
> + },
> +};
> +
> +void __init gxp_map_io(void)
> +{
> + iotable_init(gxp_io_desc, ARRAY_SIZE(gxp_io_desc));
> +}
> +
> +static void __init gxp_dt_init(void)
> +{
> + /*reset EHCI host controller for clear start*/
> + __raw_writel(0x00080002,
> + (void __iomem *)(IOP_REGS_VIRT_BASE + IOP_EHCI_USBCMD));
Please consider making IOP_REGS_VIRT_BASE a 'void __iomem' pointer, it
being a _virtual_ iomem address. This should save you needing repeated
casts except for the initialiser above.
> + of_platform_populate(NULL, of_default_bus_match_table, NULL, NULL);
> +}
> +
> +static void gxp_restart(enum reboot_mode mode, const char *cmd)
> +{
> + __raw_writel(1, (void __iomem *) IOP_REGS_VIRT_BASE);
> +}
> +
> +static const char * const gxp_board_dt_compat[] = {
> + "HPE,GXP",
> + NULL,
> +};
> +
> +DT_MACHINE_START(GXP_DT, "HPE GXP")
> + .init_machine = gxp_dt_init,
> + .map_io = gxp_map_io,
> + .restart= gxp_restart,
> + .dt_compat = gxp_board_dt_compat,
> +MACHINE_END
> diff --git a/drivers/clocksource/Kconfig b/drivers/clocksource/Kconfig
> index cfb8ea0df3b1..5916dade7608 100644
> --- a/drivers/clocksource/Kconfig
> +++ b/drivers/clocksource/Kconfig
> @@ -617,6 +617,14 @@ config CLKSRC_ST_LPC
> Enable this option to use the Low Power controller timer
> as clocksource.
>
> +config GXP_TIMER
> + bool "GXP timer driver"
> + depends on ARCH_HPE
> + default y
> + help
> + Provides a driver for the timer control found on HPE
> + GXP SOCs. This is required for all GXP SOCs.
> +
> config ATCPIT100_TIMER
> bool "ATCPIT100 timer driver"
> depends on NDS32 || COMPILE_TEST
> diff --git a/drivers/clocksource/Makefile b/drivers/clocksource/Makefile
> index fa5f624eadb6..ffca09ec34de 100644
> --- a/drivers/clocksource/Makefile
> +++ b/drivers/clocksource/Makefile
> @@ -89,3 +89,4 @@ obj-$(CONFIG_GX6605S_TIMER) += timer-gx6605s.o
> obj-$(CONFIG_HYPERV_TIMER) += hyperv_timer.o
> obj-$(CONFIG_MICROCHIP_PIT64B) += timer-microchip-pit64b.o
> obj-$(CONFIG_MSC313E_TIMER) += timer-msc313e.o
> +obj-$(CONFIG_GXP_TIMER) += gxp_timer.o
> diff --git a/drivers/clocksource/gxp_timer.c b/drivers/clocksource/gxp_timer.c
> new file mode 100644
> index ..e3c617036e0d
> --- /dev/null
> +++ b/drivers/clocksource/gxp_timer.c
> @@ -0,0 +1,158 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/* Copyright (C) 2022 Hewlett-Packard Enterprise Development Company, L.P.
> + *
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License version 2 as
> + * published by the Free Software Foundation.
> + */
> +
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +
> +#include
Why do you need asm/irq.h ?
> +
> +#define TIMER0_FREQ 100
> +#define TIMER1_FREQ 100
> +
> +#define MASK_TCS_ENABLE 0x01
> +#define MASK_TCS_PERIOD 0x02
> +#define MASK_TCS_RELOAD 0x04
> +#define MASK_TCS_TC 0x80
> +
> +struct gxp_timer {
> + void __iomem *counter;
> + void __iomem *control;
> + struct clock_event_device evt;
> +};
> +
> +static void __iomem *system_clock __read_mostly;
> +
> +static u64 notrace
Re: [PATCH] HPE BMC GXP SUPPORT
On 03/02/2022 18:07, Verdun, Jean-Marie wrote: > >> Maybe it does not look like, but this is actually a v2. Nick was asked >> to change the naming for the nodes already in v1. Unfortunately it did >> not happen, so we have vuart, spifi, vic and more. > >> It is a waste of reviewers' time to ask them to perform the same review >> twice or to ignore their comments. > > Hi Krysztof, > > Accept our apologies if you think you lose your time, it is clearly not > our > intent. > > This is the first time that we (I mean the team) introduce a new arch into > the linux kernel and I must admit that we had hard time to understand > from which angle we needed to start. > > I will probably write a Documentation afterward, as it is easy to find doc > on how to introduce a patch or a driver, but not when you want to > introduce a new chip. > > We are trying to do our best, and clearly want to follow all of your > inputs. > Mistakes happen and they are clearly not intentional and not driven in > a way to make you lose your time. > > Helping others, and teaching something new is definitely a way to > optimize your time and this is what you are currently doing with us. > > We appreciate it and hope you will too. I understand, I also maybe over-reacted on this. Just please go through the comments you got for first submission and either apply them or respond why you disagree. The next submissions (patchset split into several commits) should be a v3, preferably with cover letter (git format-patch --cover-letter -v3 ...) where you can document also changes you did to the patchset. It looks for example like this: https://lore.kernel.org/linux-samsung-soc/31da451b-a36c-74fb-5667-d4193284c...@canonical.com/T/#mf98d2ac27a8481dc69dd110f9861c8318cade252 or like this (where changelogs are in each patch, although ordering is not correct because dt-bindings should be the first in the series): https://lore.kernel.org/all/20220103233948.198119-1-mr.bossman...@gmail.com/ Best regards, Krzysztof
Re: [PATCH] HPE BMC GXP SUPPORT
> Maybe it does not look like, but this is actually a v2. Nick was asked > to change the naming for the nodes already in v1. Unfortunately it did > not happen, so we have vuart, spifi, vic and more. > It is a waste of reviewers' time to ask them to perform the same review > twice or to ignore their comments. Hi Krysztof, Accept our apologies if you think you lose your time, it is clearly not our intent. This is the first time that we (I mean the team) introduce a new arch into the linux kernel and I must admit that we had hard time to understand from which angle we needed to start. I will probably write a Documentation afterward, as it is easy to find doc on how to introduce a patch or a driver, but not when you want to introduce a new chip. We are trying to do our best, and clearly want to follow all of your inputs. Mistakes happen and they are clearly not intentional and not driven in a way to make you lose your time. Helping others, and teaching something new is definitely a way to optimize your time and this is what you are currently doing with us. We appreciate it and hope you will too. vejmarie > Best regards, > Krzysztof
Re: [PATCH] HPE BMC GXP SUPPORT
On 03/02/2022 15:29, Rob Herring wrote:
> On Wed, Feb 2, 2022 at 10:55 AM wrote:
>>
>> From: Nick Hawkins
>>
(...)
>> +
>> + vuart_a: vuart_a@80fd0200 {
>
> serial@...
Maybe it does not look like, but this is actually a v2. Nick was asked
to change the naming for the nodes already in v1. Unfortunately it did
not happen, so we have vuart, spifi, vic and more.
It is a waste of reviewers' time to ask them to perform the same review
twice or to ignore their comments.
>
>> + compatible = "hpe,gxp-vuart";
>> + reg = <0x80fd0200 0x100>;
>> + interrupts = <2>;
>> + interrupt-parent = <>;
>> + clock-frequency = <1846153>;
>> + reg-shift = <0>;
>> + status = "okay";
>> + serial-line = <3>;
>> + vuart_cfg = <_a_cfg>;
>> + };
(...)
>> diff --git a/Documentation/devicetree/bindings/vendor-prefixes.yaml
>> b/Documentation/devicetree/bindings/vendor-prefixes.yaml
>> index 294093d45a23..913f722a6b8d 100644
>> --- a/Documentation/devicetree/bindings/vendor-prefixes.yaml
>> +++ b/Documentation/devicetree/bindings/vendor-prefixes.yaml
>> @@ -514,7 +514,9 @@ patternProperties:
>>"^hoperun,.*":
>> description: Jiangsu HopeRun Software Co., Ltd.
>>"^hp,.*":
>> -description: Hewlett Packard
>> +description: Hewlett Packard Inc.
>
> Why are you changing this one?
I guess this is squashing of my patch:
https://lore.kernel.org/all/20220127075229.10299-1-krzysztof.kozlow...@canonical.com/
which is fine to me, but vendor changve should be a separate commit with
its own explanation. Now it looks indeed weird.
>
>> + "^hpe,.*":
>
> You used HPE elsewhere... Lowercase is preferred.
Best regards,
Krzysztof
Re: [PATCH] HPE BMC GXP SUPPORT
On Wed, Feb 2, 2022 at 10:55 AM wrote: > > From: Nick Hawkins > > GXP is the name of the HPE SoC. > This SoC is used to implement BMC features of HPE servers > (all ProLiant, Synergy, and many Apollo, and Superdome machines) > It does support many features including: > ARMv7 architecture, and it is based on a Cortex A9 core > Use an AXI bus to which > a memory controller is attached, as well as > multiple SPI interfaces to connect boot flash, > and ROM flash, a 10/100/1000 Mac engine which > supports SGMII (2 ports) and RMII > Multiple I2C engines to drive connectivity with a host > infrastructure > A video engine which support VGA and DP, as well as > an hardware video encoder > Multiple PCIe ports > A PECI interface, and LPC eSPI > Multiple UART for debug purpose, and Virtual UART for host > connectivity > A GPIO engine > This Patch Includes: > Documentation for device tree bindings > Device Tree Bindings > GXP Timer Support > GXP Architecture Support > > Signed-off-by: Nick Hawkins > --- > .../bindings/display/hpe,gxp-thumbnail.txt| 21 + > .../devicetree/bindings/gpio/hpe,gxp-gpio.txt | 16 + > .../devicetree/bindings/i2c/hpe,gxp-i2c.txt | 19 + > .../bindings/ipmi/hpegxp-kcs-bmc-cfg.txt | 13 + > .../bindings/ipmi/hpegxp-kcs-bmc.txt | 21 + > .../memory-controllers/hpe,gxp-srom.txt | 13 + > .../devicetree/bindings/mtd/hpe,gxp.txt | 16 + > .../bindings/net/hpe,gxp-umac-mdio.txt| 21 + > .../devicetree/bindings/net/hpe,gxp-umac.txt | 20 + > .../devicetree/bindings/pwm/hpe,gxp-fan.txt | 15 + > .../bindings/serial/hpe,gxp-vuart-cfg.txt | 17 + > .../bindings/serial/hpe,gxp-vuart.txt | 23 + > .../bindings/soc/hpe/hpe,gxp-chif.txt | 16 + > .../bindings/soc/hpe/hpe,gxp-csm.txt | 14 + > .../bindings/soc/hpe/hpe,gxp-dbg.txt | 18 + > .../bindings/soc/hpe/hpe,gxp-fn2.txt | 20 + > .../bindings/soc/hpe/hpe,gxp-xreg.txt | 19 + > .../devicetree/bindings/spi/hpe,gxp-spifi.txt | 76 +++ > .../bindings/thermal/hpe,gxp-coretemp.txt | 14 + > .../bindings/timer/hpe,gxp-timer.txt | 18 + > .../devicetree/bindings/usb/hpe,gxp-udc.txt | 21 + > .../devicetree/bindings/vendor-prefixes.yaml | 4 +- > .../bindings/watchdog/hpe,gxp-wdt.txt | 11 + All these must be in DT schema format (yaml json-schema). See Documentation/devicetree/bindings/submitting-patches.rst and Documentation/devicetree/bindings/writing-schema.rst. > MAINTAINERS | 14 + > arch/arm/Kconfig | 2 + > arch/arm/boot/dts/Makefile| 2 + > arch/arm/boot/dts/hpe-bmc-dl360gen10.dts | 207 +++ > arch/arm/boot/dts/hpe-gxp.dtsi| 555 ++ Once the schemas are in place, run 'make W=1 dtbs_check' and fix the warnings. > arch/arm/configs/gxp_defconfig| 243 > arch/arm/mach-hpe/Kconfig | 21 + > arch/arm/mach-hpe/Makefile| 1 + > arch/arm/mach-hpe/gxp.c | 62 ++ > drivers/clocksource/Kconfig | 8 + > drivers/clocksource/Makefile | 1 + > drivers/clocksource/gxp_timer.c | 158 + > 35 files changed, 1719 insertions(+), 1 deletion(-) > create mode 100644 > Documentation/devicetree/bindings/display/hpe,gxp-thumbnail.txt > create mode 100644 Documentation/devicetree/bindings/gpio/hpe,gxp-gpio.txt > create mode 100644 Documentation/devicetree/bindings/i2c/hpe,gxp-i2c.txt > create mode 100644 > Documentation/devicetree/bindings/ipmi/hpegxp-kcs-bmc-cfg.txt > create mode 100644 Documentation/devicetree/bindings/ipmi/hpegxp-kcs-bmc.txt > create mode 100644 > Documentation/devicetree/bindings/memory-controllers/hpe,gxp-srom.txt > create mode 100644 Documentation/devicetree/bindings/mtd/hpe,gxp.txt > create mode 100644 > Documentation/devicetree/bindings/net/hpe,gxp-umac-mdio.txt > create mode 100644 Documentation/devicetree/bindings/net/hpe,gxp-umac.txt > create mode 100644 Documentation/devicetree/bindings/pwm/hpe,gxp-fan.txt > create mode 100644 > Documentation/devicetree/bindings/serial/hpe,gxp-vuart-cfg.txt > create mode 100644 Documentation/devicetree/bindings/serial/hpe,gxp-vuart.txt > create mode 100644 Documentation/devicetree/bindings/soc/hpe/hpe,gxp-chif.txt > create mode 100644 Documentation/devicetree/bindings/soc/hpe/hpe,gxp-csm.txt > create mode 100644 Documentation/devicetree/bindings/soc/hpe/hpe,gxp-dbg.txt > create mode 100644 Documentation/devicetree/bindings/soc/hpe/hpe,gxp-fn2.txt > create mode 100644 Documentation/devicetree/bindings/soc/hpe/hpe,gxp-xreg.txt > create mode 100644
Re: [PATCH] HPE BMC GXP SUPPORT
On 02/02/2022 17:52, nick.hawk...@hpe.com wrote: > From: Nick Hawkins > > GXP is the name of the HPE SoC. > This SoC is used to implement BMC features of HPE servers > (all ProLiant, Synergy, and many Apollo, and Superdome machines) > It does support many features including: > ARMv7 architecture, and it is based on a Cortex A9 core > Use an AXI bus to which > a memory controller is attached, as well as > multiple SPI interfaces to connect boot flash, > and ROM flash, a 10/100/1000 Mac engine which > supports SGMII (2 ports) and RMII > Multiple I2C engines to drive connectivity with a host > infrastructure > A video engine which support VGA and DP, as well as > an hardware video encoder > Multiple PCIe ports > A PECI interface, and LPC eSPI > Multiple UART for debug purpose, and Virtual UART for host > connectivity > A GPIO engine > This Patch Includes: > Documentation for device tree bindings > Device Tree Bindings > GXP Timer Support > GXP Architecture Support > 1. Please version your patchses and document the changes under ---. 2. With your v1 I responded what has to be separate patch. This was totally ignored here, so no. You have to follow this. 3. Please run checkpatch and be sure there are no warnings. 4. Bindings in dtschema, not in text. Best regards, Krzysztof Best regards, Krzysztof
Re: [PATCH] HPE BMC GXP SUPPORT
> .../bindings/display/hpe,gxp-thumbnail.txt| 21 + > .../devicetree/bindings/gpio/hpe,gxp-gpio.txt | 16 + > .../devicetree/bindings/i2c/hpe,gxp-i2c.txt | 19 + > .../bindings/ipmi/hpegxp-kcs-bmc-cfg.txt | 13 + > .../bindings/ipmi/hpegxp-kcs-bmc.txt | 21 + Hi Nick In addiiton to the other feedback also given, for new bindings you should be using yaml, not txt. You then gain validation of the bindings. Andrew
Re: [PATCH] HPE BMC GXP SUPPORT
On Wed, Feb 02, 2022 at 10:52:50AM -0600, nick.hawk...@hpe.com wrote: > From: Nick Hawkins > > GXP is the name of the HPE SoC. > This SoC is used to implement BMC features of HPE servers > (all ProLiant, Synergy, and many Apollo, and Superdome machines) > It does support many features including: > ARMv7 architecture, and it is based on a Cortex A9 core > Use an AXI bus to which > a memory controller is attached, as well as > multiple SPI interfaces to connect boot flash, > and ROM flash, a 10/100/1000 Mac engine which > supports SGMII (2 ports) and RMII > Multiple I2C engines to drive connectivity with a host > infrastructure > A video engine which support VGA and DP, as well as > an hardware video encoder > Multiple PCIe ports > A PECI interface, and LPC eSPI > Multiple UART for debug purpose, and Virtual UART for host > connectivity > A GPIO engine > This Patch Includes: > Documentation for device tree bindings > Device Tree Bindings > GXP Timer Support > GXP Architecture Support This is far too big for a single patch. It needs to be broken into functional chunks that can be reviewed individually. Each driver and each device tree change along with it's accompanying code need to be done in individual patches. The way it is it can't be reviewed in any sane manner. -corey > > Signed-off-by: Nick Hawkins > --- > .../bindings/display/hpe,gxp-thumbnail.txt| 21 + > .../devicetree/bindings/gpio/hpe,gxp-gpio.txt | 16 + > .../devicetree/bindings/i2c/hpe,gxp-i2c.txt | 19 + > .../bindings/ipmi/hpegxp-kcs-bmc-cfg.txt | 13 + > .../bindings/ipmi/hpegxp-kcs-bmc.txt | 21 + > .../memory-controllers/hpe,gxp-srom.txt | 13 + > .../devicetree/bindings/mtd/hpe,gxp.txt | 16 + > .../bindings/net/hpe,gxp-umac-mdio.txt| 21 + > .../devicetree/bindings/net/hpe,gxp-umac.txt | 20 + > .../devicetree/bindings/pwm/hpe,gxp-fan.txt | 15 + > .../bindings/serial/hpe,gxp-vuart-cfg.txt | 17 + > .../bindings/serial/hpe,gxp-vuart.txt | 23 + > .../bindings/soc/hpe/hpe,gxp-chif.txt | 16 + > .../bindings/soc/hpe/hpe,gxp-csm.txt | 14 + > .../bindings/soc/hpe/hpe,gxp-dbg.txt | 18 + > .../bindings/soc/hpe/hpe,gxp-fn2.txt | 20 + > .../bindings/soc/hpe/hpe,gxp-xreg.txt | 19 + > .../devicetree/bindings/spi/hpe,gxp-spifi.txt | 76 +++ > .../bindings/thermal/hpe,gxp-coretemp.txt | 14 + > .../bindings/timer/hpe,gxp-timer.txt | 18 + > .../devicetree/bindings/usb/hpe,gxp-udc.txt | 21 + > .../devicetree/bindings/vendor-prefixes.yaml | 4 +- > .../bindings/watchdog/hpe,gxp-wdt.txt | 11 + > MAINTAINERS | 14 + > arch/arm/Kconfig | 2 + > arch/arm/boot/dts/Makefile| 2 + > arch/arm/boot/dts/hpe-bmc-dl360gen10.dts | 207 +++ > arch/arm/boot/dts/hpe-gxp.dtsi| 555 ++ > arch/arm/configs/gxp_defconfig| 243 > arch/arm/mach-hpe/Kconfig | 21 + > arch/arm/mach-hpe/Makefile| 1 + > arch/arm/mach-hpe/gxp.c | 62 ++ > drivers/clocksource/Kconfig | 8 + > drivers/clocksource/Makefile | 1 + > drivers/clocksource/gxp_timer.c | 158 + > 35 files changed, 1719 insertions(+), 1 deletion(-) > create mode 100644 > Documentation/devicetree/bindings/display/hpe,gxp-thumbnail.txt > create mode 100644 Documentation/devicetree/bindings/gpio/hpe,gxp-gpio.txt > create mode 100644 Documentation/devicetree/bindings/i2c/hpe,gxp-i2c.txt > create mode 100644 > Documentation/devicetree/bindings/ipmi/hpegxp-kcs-bmc-cfg.txt > create mode 100644 Documentation/devicetree/bindings/ipmi/hpegxp-kcs-bmc.txt > create mode 100644 > Documentation/devicetree/bindings/memory-controllers/hpe,gxp-srom.txt > create mode 100644 Documentation/devicetree/bindings/mtd/hpe,gxp.txt > create mode 100644 > Documentation/devicetree/bindings/net/hpe,gxp-umac-mdio.txt > create mode 100644 Documentation/devicetree/bindings/net/hpe,gxp-umac.txt > create mode 100644 Documentation/devicetree/bindings/pwm/hpe,gxp-fan.txt > create mode 100644 > Documentation/devicetree/bindings/serial/hpe,gxp-vuart-cfg.txt > create mode 100644 Documentation/devicetree/bindings/serial/hpe,gxp-vuart.txt > create mode 100644 Documentation/devicetree/bindings/soc/hpe/hpe,gxp-chif.txt > create mode 100644 Documentation/devicetree/bindings/soc/hpe/hpe,gxp-csm.txt > create mode 100644 Documentation/devicetree/bindings/soc/hpe/hpe,gxp-dbg.txt > create mode 100644 Documentation/devicetree/bindings/soc/hpe/hpe,gxp-fn2.txt > create mode 100644
Re: [PATCH] HPE BMC GXP SUPPORT
> This is far too big for a single patch. It needs to be broken into > functional chunks that can be reviewed individually. Each driver and > each device tree change along with it's accompanying code need to be > done in individual patches. The way it is it can't be reviewed in any > sane manner. > -corey Thanks for your feedback. We are getting a little bit lost here, as our plan was to submit initial - bindings - dts for SoC and 1 board - initial platform init code Then drivers code avoiding to send many dts updates which might complexify the review. We wanted to send all drivers code to relevant reviewers by tomorrow. So, what you are asking ( do not worry I am not trying to negotiate, I just want to avoid English misunderstandings as I am French) is to send per driver - binding - dts update - driver code For each driver through different submission (with each of them containing the 3 associated parts) ? What shall be the initial one in our case as we are introducing a platform ? An empty dts infrastructure and then we make it grow one step at a time ? vejmarie
Re: [Openipmi-developer] [PATCH] HPE BMC GXP SUPPORT
On Wed, Feb 02, 2022 at 06:14:57PM +, Verdun, Jean-Marie wrote: > > This is far too big for a single patch. It needs to be broken into > > functional chunks that can be reviewed individually. Each driver and > > each device tree change along with it's accompanying code need to be > > done in individual patches. The way it is it can't be reviewed in any > > sane manner. > > > -corey > > Thanks for your feedback. We are getting a little bit lost here, as our plan > was to submit initial > > - bindings > - dts for SoC and 1 board > - initial platform init code > > Then drivers code avoiding to send many dts updates which might complexify > the review. We wanted to send all drivers code to relevant reviewers by > tomorrow. > > So, what you are asking ( do not worry I am not trying to negotiate, I just > want to avoid English misunderstandings as I am French) is to send per driver > > - binding > - dts update > - driver code > > For each driver through different submission (with each of them containing > the 3 associated parts) ? Arnd gave an excellent explaination for this. To be clear, you need to split out changes to individual subsystems and submit those to the maintainers for that subsystem and not send them to everyone. That way you reduce sending emails to people who don't need to see them. Once you have a set of patches for a subsystem, you can submit them as one set. That is generally preferred. The "git send-email" or "git format-patch" tools are generally what we use, they let you compose a header message where you can give an overall explaination, then it sends the individual changes as followup messages to the header message. -corey > > What shall be the initial one in our case as we are introducing a platform ? > An empty dts infrastructure and then we make it grow one step at a time ? > > vejmarie > > > > > > ___ > Openipmi-developer mailing list > openipmi-develo...@lists.sourceforge.net > https://lists.sourceforge.net/lists/listinfo/openipmi-developer
Re: [PATCH] HPE BMC GXP SUPPORT
On Wed, Feb 2, 2022 at 7:14 PM Verdun, Jean-Marie wrote: > > > This is far too big for a single patch. It needs to be broken into > > functional chunks that can be reviewed individually. Each driver and > > each device tree change along with it's accompanying code need to be > > done in individual patches. The way it is it can't be reviewed in any > > sane manner. > > > -corey > > Thanks for your feedback. We are getting a little bit lost here, as our plan > was to submit initial > > - bindings > - dts for SoC and 1 board > - initial platform init code > > Then drivers code avoiding to send many dts updates which might complexify the > review. We wanted to send all drivers code to relevant reviewers by tomorrow. > > So, what you are asking ( do not worry I am not trying to negotiate, I just > want > to avoid English misunderstandings as I am French) is to send per driver > > - binding > - dts update > - driver code > > For each driver through different submission (with each of them containing the > 3 associated parts) ? > > What shall be the initial one in our case as we are introducing a platform ? > An empty dts infrastructure and then we make it grow one step at a time ? Ideally, what I prefer to see is a series of patches for all "essential" drivers and the platform code that includes: - one patch for each new binding - one patch for each new driver - one patch that hooks up arch/arm/mach-hpe/, MAINTAINERS and any other changes to arch/arm/ other than dts - one patch that adds the initial .dts and .dtsi files, with all the devices added that have a valid binding, no need to split this up any further This should include everything you need to boot into an initramfs shell, typically cpu, serial, timer, clk, pinctrl, gpio, irqchip. We will merge these as a git branch in the soc tree. In parallel, you can work with subsystem maintainers for the "non-essential" drivers to review any other driver and binding, e.g. drm/kms, network, i2c, pci, usb, etc. The patches for the corresponding .dts additions also go through the soc tree, but to make things simpler, you can send those in for a later release. Arnd
Re: [PATCH] HPE BMC GXP SUPPORT
Hi Nick, good to see all this stuff coming mainline, On Wed, Feb 02, 2022 at 10:52:50AM -0600, nick.hawk...@hpe.com wrote: > From: Nick Hawkins > > GXP is the name of the HPE SoC. > This SoC is used to implement BMC features of HPE servers > (all ProLiant, Synergy, and many Apollo, and Superdome machines) > It does support many features including: > ARMv7 architecture, and it is based on a Cortex A9 core > Use an AXI bus to which > a memory controller is attached, as well as > multiple SPI interfaces to connect boot flash, > and ROM flash, a 10/100/1000 Mac engine which > supports SGMII (2 ports) and RMII > Multiple I2C engines to drive connectivity with a host > infrastructure > A video engine which support VGA and DP, as well as > an hardware video encoder > Multiple PCIe ports > A PECI interface, and LPC eSPI > Multiple UART for debug purpose, and Virtual UART for host > connectivity > A GPIO engine > This Patch Includes: > Documentation for device tree bindings > Device Tree Bindings > GXP Timer Support > GXP Architecture Support > > Signed-off-by: Nick Hawkins > --- > .../bindings/display/hpe,gxp-thumbnail.txt| 21 + > .../devicetree/bindings/gpio/hpe,gxp-gpio.txt | 16 + ... All new bindings must be in the DT-schema format (yaml files). This enables a lot of syntax checks and validation. We are slowly migrating away from the .txt based bindings. Also, for new bindings please follow the guide lines listed in Documentation/devicetree/bindings/submitting-patches.rst Consider including the bindings with the drivers using the bindings so things have a more natural split. Sam
[PATCH] HPE BMC GXP SUPPORT
From: Nick Hawkins GXP is the name of the HPE SoC. This SoC is used to implement BMC features of HPE servers (all ProLiant, Synergy, and many Apollo, and Superdome machines) It does support many features including: ARMv7 architecture, and it is based on a Cortex A9 core Use an AXI bus to which a memory controller is attached, as well as multiple SPI interfaces to connect boot flash, and ROM flash, a 10/100/1000 Mac engine which supports SGMII (2 ports) and RMII Multiple I2C engines to drive connectivity with a host infrastructure A video engine which support VGA and DP, as well as an hardware video encoder Multiple PCIe ports A PECI interface, and LPC eSPI Multiple UART for debug purpose, and Virtual UART for host connectivity A GPIO engine This Patch Includes: Documentation for device tree bindings Device Tree Bindings GXP Timer Support GXP Architecture Support Signed-off-by: Nick Hawkins --- .../bindings/display/hpe,gxp-thumbnail.txt| 21 + .../devicetree/bindings/gpio/hpe,gxp-gpio.txt | 16 + .../devicetree/bindings/i2c/hpe,gxp-i2c.txt | 19 + .../bindings/ipmi/hpegxp-kcs-bmc-cfg.txt | 13 + .../bindings/ipmi/hpegxp-kcs-bmc.txt | 21 + .../memory-controllers/hpe,gxp-srom.txt | 13 + .../devicetree/bindings/mtd/hpe,gxp.txt | 16 + .../bindings/net/hpe,gxp-umac-mdio.txt| 21 + .../devicetree/bindings/net/hpe,gxp-umac.txt | 20 + .../devicetree/bindings/pwm/hpe,gxp-fan.txt | 15 + .../bindings/serial/hpe,gxp-vuart-cfg.txt | 17 + .../bindings/serial/hpe,gxp-vuart.txt | 23 + .../bindings/soc/hpe/hpe,gxp-chif.txt | 16 + .../bindings/soc/hpe/hpe,gxp-csm.txt | 14 + .../bindings/soc/hpe/hpe,gxp-dbg.txt | 18 + .../bindings/soc/hpe/hpe,gxp-fn2.txt | 20 + .../bindings/soc/hpe/hpe,gxp-xreg.txt | 19 + .../devicetree/bindings/spi/hpe,gxp-spifi.txt | 76 +++ .../bindings/thermal/hpe,gxp-coretemp.txt | 14 + .../bindings/timer/hpe,gxp-timer.txt | 18 + .../devicetree/bindings/usb/hpe,gxp-udc.txt | 21 + .../devicetree/bindings/vendor-prefixes.yaml | 4 +- .../bindings/watchdog/hpe,gxp-wdt.txt | 11 + MAINTAINERS | 14 + arch/arm/Kconfig | 2 + arch/arm/boot/dts/Makefile| 2 + arch/arm/boot/dts/hpe-bmc-dl360gen10.dts | 207 +++ arch/arm/boot/dts/hpe-gxp.dtsi| 555 ++ arch/arm/configs/gxp_defconfig| 243 arch/arm/mach-hpe/Kconfig | 21 + arch/arm/mach-hpe/Makefile| 1 + arch/arm/mach-hpe/gxp.c | 62 ++ drivers/clocksource/Kconfig | 8 + drivers/clocksource/Makefile | 1 + drivers/clocksource/gxp_timer.c | 158 + 35 files changed, 1719 insertions(+), 1 deletion(-) create mode 100644 Documentation/devicetree/bindings/display/hpe,gxp-thumbnail.txt create mode 100644 Documentation/devicetree/bindings/gpio/hpe,gxp-gpio.txt create mode 100644 Documentation/devicetree/bindings/i2c/hpe,gxp-i2c.txt create mode 100644 Documentation/devicetree/bindings/ipmi/hpegxp-kcs-bmc-cfg.txt create mode 100644 Documentation/devicetree/bindings/ipmi/hpegxp-kcs-bmc.txt create mode 100644 Documentation/devicetree/bindings/memory-controllers/hpe,gxp-srom.txt create mode 100644 Documentation/devicetree/bindings/mtd/hpe,gxp.txt create mode 100644 Documentation/devicetree/bindings/net/hpe,gxp-umac-mdio.txt create mode 100644 Documentation/devicetree/bindings/net/hpe,gxp-umac.txt create mode 100644 Documentation/devicetree/bindings/pwm/hpe,gxp-fan.txt create mode 100644 Documentation/devicetree/bindings/serial/hpe,gxp-vuart-cfg.txt create mode 100644 Documentation/devicetree/bindings/serial/hpe,gxp-vuart.txt create mode 100644 Documentation/devicetree/bindings/soc/hpe/hpe,gxp-chif.txt create mode 100644 Documentation/devicetree/bindings/soc/hpe/hpe,gxp-csm.txt create mode 100644 Documentation/devicetree/bindings/soc/hpe/hpe,gxp-dbg.txt create mode 100644 Documentation/devicetree/bindings/soc/hpe/hpe,gxp-fn2.txt create mode 100644 Documentation/devicetree/bindings/soc/hpe/hpe,gxp-xreg.txt create mode 100644 Documentation/devicetree/bindings/spi/hpe,gxp-spifi.txt create mode 100644 Documentation/devicetree/bindings/thermal/hpe,gxp-coretemp.txt create mode 100644 Documentation/devicetree/bindings/timer/hpe,gxp-timer.txt create mode 100644 Documentation/devicetree/bindings/usb/hpe,gxp-udc.txt create mode 100644 Documentation/devicetree/bindings/watchdog/hpe,gxp-wdt.txt create mode 100644 arch/arm/boot/dts/hpe-bmc-dl360gen10.dts create mode 100644
