On 8/22/22 7:25 AM, Rob Herring wrote: > On Sat, Aug 20, 2022 at 12:57:39PM -0700, Brad Larson wrote: >> From: Brad Larson <blarson@xxxxxxx> >> >> Add support for the AMD Pensando Elba SoC System Resource chip >> using the SPI interface. The Elba SR is a Multi-function Device >> supporting device register access using CS0, smbus interface for >> FRU and board peripherals using CS1, dual Lattice I2C masters for >> transceiver management using CS2, and CS3 for flash access. >> >> Signed-off-by: Brad Larson <blarson@xxxxxxx> >> --- >> .../bindings/mfd/amd,pensando-elbasr.yaml | 97 +++++++++++++++++++ >> 1 file changed, 97 insertions(+) >> create mode 100644 Documentation/devicetree/bindings/mfd/amd,pensando-elbasr.yaml >> >> diff --git a/Documentation/devicetree/bindings/mfd/amd,pensando-elbasr.yaml b/Documentation/devicetree/bindings/mfd/amd,pensando-elbasr.yaml >> new file mode 100644 >> index 000000000000..ded347c3352c >> --- /dev/null >> +++ b/Documentation/devicetree/bindings/mfd/amd,pensando-elbasr.yaml >> @@ -0,0 +1,97 @@ >> +# SPDX-License-Identifier: (GPL-2.0-only or BSD-2-Clause) >> +%YAML 1.2 >> +--- >> +$id: https://nam11.safelinks.protection.outlook.com/?url=http%3A%2F%2Fdevicetree.org%2Fschemas%2Fmfd%2Famd%2Cpensando-elbasr.yaml%23&data=05%7C01%7CBradley.Larson%40amd.com%7Cd02c183f9a29492180fb08da844a3458%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637967751571358185%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000%7C%7C%7C&sdata=WHkA6tPbaDanQuMSaAiWUG3fBTfDVlWXMdeaO5t%2F3Ok%3D&reserved=0 >> +$schema: https://nam11.safelinks.protection.outlook.com/?url=http%3A%2F%2Fdevicetree.org%2Fmeta-schemas%2Fcore.yaml%23&data=05%7C01%7CBradley.Larson%40amd.com%7Cd02c183f9a29492180fb08da844a3458%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637967751571358185%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000%7C%7C%7C&sdata=FDig31luqeo4pOZXfuOAGiOLi0kVqFU8ExnBi5gorlY%3D&reserved=0 >> + >> +title: AMD Pensando Elba SoC Resource Controller bindings >> + >> +description: | >> + AMD Pensando Elba SoC Resource Controller is a set of >> + miscellaneous control/status registers accessed on CS0, >> + a designware i2c master/slave on CS1, a Lattice rd1173 >> + dual i2c master on CS2, and flash on CS3. The /dev interfaces >> + created are /dev/pensr0.<CS>. Hardware reset of the eMMC > /dev is a Linux thing and not relevant for the bindings. > Removed mention of the dev interfaces >> + is implemented by a sub-device reset-controller which accesses >> + a CS0 control register. >> + >> +maintainers: >> + - Brad Larson <blarson@xxxxxxx> >> + >> +properties: >> + compatible: >> + items: >> + - enum: >> + - amd,pensando-elbasr >> + >> + spi-max-frequency: >> + description: Maximum SPI frequency of the device in Hz. > No need for generic descriptions of common properties. Changed to "spi-max-frequency: true" and moved to end of properties. >> + >> + reg: >> + maxItems: 1 >> + >> + '#address-cells': >> + const: 1 >> + >> + '#size-cells': >> + const: 0 >> + >> + interrupts: >> + maxItems: 1 >> + >> +required: >> + - compatible >> + - reg >> + - spi-max-frequency >> + >> +patternProperties: >> + '^reset-controller@[a-f0-9]+$': >> + $ref: /schemas/reset/amd,pensando-elbasr-reset.yaml >> + >> +additionalProperties: false >> + >> +examples: >> + - | >> + #include <dt-bindings/interrupt-controller/arm-gic.h> >> + >> + spi { >> + #address-cells = <1>; >> + #size-cells = <0>; >> + num-cs = <4>; >> + >> + sysc: system-controller@0 { >> + compatible = "amd,pensando-elbasr"; >> + reg = <0>; >> + #address-cells = <1>; >> + #size-cells = <0>; >> + spi-max-frequency = <12000000>; >> + >> + rstc: reset-controller@0 { >> + compatible = "amd,pensando-elbasr-reset"; >> + reg = <0>; > What does 0 represent here? A register address within 'elbasr' device? Removed, I recall a check threw a warning or error without reg. > Why do you need a child node for this? Are there other sub-devices and > your binding is incomplete? Just put '#reset-cells' in the parent. Without a reset-controller node and booting the function __of_reset_control_get(...) fails to find a match in the list here list_for_each_entry(r, &reset_controller_list, list) { if (args.np == r->of_node) { rcdev = r; break; } } where in sdhci_cdns_probe(...) this lookup fails priv->rst_hw = devm_reset_control_get_optional_exclusive(dev, "hw"); which results in a non-functioning mmc hardware reset. >> + #reset-cells = <1>; >> + }; >> + }; >> + >> + i2c1: i2c@1 { >> + compatible = "amd,pensando-elbasr"; > You can't reuse the same compatible to represent different things. Changed to system-controller@1 and adjusted description above >> + reg = <1>; >> + spi-max-frequency = <12000000>; >> + }; >> + >> + i2c2: i2c@2 { >> + compatible = "amd,pensando-elbasr"; > As this is a Lattice RD1173, I would expect a compatible based on that. > Same as above, changed this to system-controller@2 >> + reg = <2>; >> + spi-max-frequency = <12000000>; >> + interrupt-parent = <&porta>; >> + interrupts = <0 IRQ_TYPE_LEVEL_LOW>; >> + }; >> + >> + flash@3 { >> + compatible = "amd,pensando-elbasr"; > Isn't this a flash device? A userspace utility understands how to program this internal flash. Changed to system-controller@3 Regards, Brad
