Re: [Openipmi-developer] [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 ___ Openipmi-developer mailing list Openipmi-developer@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/openipmi-developer
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-developer@lists.sourceforge.net > https://lists.sourceforge.net/lists/listinfo/openipmi-developer ___ Openipmi-developer mailing list Openipmi-developer@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/openipmi-developer
Re: [Openipmi-developer] [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 ___ Openipmi-developer mailing list Openipmi-developer@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/openipmi-developer
Re: [Openipmi-developer] [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 ___ Openipmi-developer mailing list Openipmi-developer@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/openipmi-developer
Re: [Openipmi-developer] [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 Docu
Re: [Openipmi-developer] [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 ___ Openipmi-developer mailing list Openipmi-developer@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/openipmi-developer
Re: [Openipmi-developer] [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 = <&vic1>;
>> + clock-frequency = <1846153>;
>> + reg-shift = <0>;
>> + status = "okay";
>> + serial-line = <3>;
>> + vuart_cfg = <&vuart_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
___
Openipmi-developer mailing list
Openipmi-developer@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openipmi-developer
