* [PATCH 0/3] occ: Restore default behavior of polling OCC during init @ 2022-08-02 19:46 Eddie James 2022-08-02 19:46 ` [PATCH 1/3] dt-bindings: hwmon: Add IBM OCC bindings Eddie James ` (2 more replies) 0 siblings, 3 replies; 11+ messages in thread From: Eddie James @ 2022-08-02 19:46 UTC (permalink / raw) To: joel Cc: linux, jdelvare, robh+dt, krzysztof.kozlowski+dt, linux-kernel, linux-hwmon, linux-fsi, devicetree, Eddie James A recent change to the OCC hwmon driver modified the default behavior to no longer poll the OCC during initialization. This does change the interface, meaning that old applications will not work with the more recent driver. To resolve this issue, introduce a new dts property to control the behavior of the driver during initialization, similar to the FSI master property "no-scan-on-init". Without the new "ibm,inactive-on-init" boolean present, the driver will now do the previous behavior of polling the OCC. Eddie James (3): dt-bindings: hwmon: Add IBM OCC bindings fsi: occ: Support probing the hwmon child device from dts node hwmon: (occ) Check for device property for setting OCC active during probe .../bindings/hwmon/ibm,occ-hmwon.yaml | 40 ++++++++++++++++++ drivers/fsi/fsi-occ.c | 41 +++++++++++++++---- drivers/hwmon/occ/common.c | 11 ++++- drivers/hwmon/occ/p9_sbe.c | 9 ++++ 4 files changed, 93 insertions(+), 8 deletions(-) create mode 100644 Documentation/devicetree/bindings/hwmon/ibm,occ-hmwon.yaml -- 2.31.1 ^ permalink raw reply [flat|nested] 11+ messages in thread
* [PATCH 1/3] dt-bindings: hwmon: Add IBM OCC bindings 2022-08-02 19:46 [PATCH 0/3] occ: Restore default behavior of polling OCC during init Eddie James @ 2022-08-02 19:46 ` Eddie James 2022-08-02 22:38 ` Rob Herring 2022-08-03 6:55 ` Krzysztof Kozlowski 2022-08-02 19:46 ` [PATCH 2/3] fsi: occ: Support probing the hwmon child device from dts node Eddie James 2022-08-02 19:46 ` [PATCH 3/3] hwmon: (occ) Check for device property for setting OCC active during probe Eddie James 2 siblings, 2 replies; 11+ messages in thread From: Eddie James @ 2022-08-02 19:46 UTC (permalink / raw) To: joel Cc: linux, jdelvare, robh+dt, krzysztof.kozlowski+dt, linux-kernel, linux-hwmon, linux-fsi, devicetree, Eddie James These bindings describe the POWER processor On Chip Controller accessed from a service processor or baseboard management controller (BMC). Signed-off-by: Eddie James <eajames@linux.ibm.com> --- .../bindings/hwmon/ibm,occ-hmwon.yaml | 40 +++++++++++++++++++ 1 file changed, 40 insertions(+) create mode 100644 Documentation/devicetree/bindings/hwmon/ibm,occ-hmwon.yaml diff --git a/Documentation/devicetree/bindings/hwmon/ibm,occ-hmwon.yaml b/Documentation/devicetree/bindings/hwmon/ibm,occ-hmwon.yaml new file mode 100644 index 000000000000..8f8c3b8d7129 --- /dev/null +++ b/Documentation/devicetree/bindings/hwmon/ibm,occ-hmwon.yaml @@ -0,0 +1,40 @@ +# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause +%YAML 1.2 +--- +$id: http://devicetree.org/schemas/hwmon/ibm,occ-hwmon.yaml# +$schema: http://devicetree.org/meta-schemas/core.yaml# + +title: IBM On-Chip Controller (OCC) accessed from a service processor + +maintainers: + - Eddie James <eajames@linux.ibm.com> + +description: | + This binding describes a POWER processor On-Chip Controller (OCC) + accessed from a service processor or baseboard management controller + (BMC). + +properties: + compatible: + enum: + - ibm,p9-occ-hwmon + - ibm,p10-occ-hwmon + + ibm,inactive-on-init: + description: This property describes whether or not the OCC should + be marked as active during device initialization. The alternative + is for user space to mark the device active based on higher level + communications between the BMC and the host processor. + type: boolean + +required: + - compatible + +additionalProperties: false + +examples: + - | + occ-hmwon { + compatible = "ibm,p9-occ-hwmon"; + ibm,inactive-on-init; + }; -- 2.31.1 ^ permalink raw reply related [flat|nested] 11+ messages in thread
* Re: [PATCH 1/3] dt-bindings: hwmon: Add IBM OCC bindings 2022-08-02 19:46 ` [PATCH 1/3] dt-bindings: hwmon: Add IBM OCC bindings Eddie James @ 2022-08-02 22:38 ` Rob Herring 2022-08-03 6:55 ` Krzysztof Kozlowski 1 sibling, 0 replies; 11+ messages in thread From: Rob Herring @ 2022-08-02 22:38 UTC (permalink / raw) To: Eddie James Cc: linux-kernel, robh+dt, linux-fsi, linux-hwmon, linux, devicetree, jdelvare, joel, krzysztof.kozlowski+dt On Tue, 02 Aug 2022 14:46:54 -0500, Eddie James wrote: > These bindings describe the POWER processor On Chip Controller accessed > from a service processor or baseboard management controller (BMC). > > Signed-off-by: Eddie James <eajames@linux.ibm.com> > --- > .../bindings/hwmon/ibm,occ-hmwon.yaml | 40 +++++++++++++++++++ > 1 file changed, 40 insertions(+) > create mode 100644 Documentation/devicetree/bindings/hwmon/ibm,occ-hmwon.yaml > My bot found errors running 'make DT_CHECKER_FLAGS=-m dt_binding_check' on your patch (DT_CHECKER_FLAGS is new in v5.13): yamllint warnings/errors: dtschema/dtc warnings/errors: ./Documentation/devicetree/bindings/hwmon/ibm,occ-hmwon.yaml: $id: relative path/filename doesn't match actual path or filename expected: http://devicetree.org/schemas/hwmon/ibm,occ-hmwon.yaml# doc reference errors (make refcheckdocs): See https://patchwork.ozlabs.org/patch/ This check can fail if there are any dependencies. The base for a patch series is generally the most recent rc1. If you already ran 'make dt_binding_check' and didn't see the above error(s), then make sure 'yamllint' is installed and dt-schema is up to date: pip3 install dtschema --upgrade Please check and re-submit. ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 1/3] dt-bindings: hwmon: Add IBM OCC bindings 2022-08-02 19:46 ` [PATCH 1/3] dt-bindings: hwmon: Add IBM OCC bindings Eddie James 2022-08-02 22:38 ` Rob Herring @ 2022-08-03 6:55 ` Krzysztof Kozlowski 2022-08-09 19:42 ` Eddie James 1 sibling, 1 reply; 11+ messages in thread From: Krzysztof Kozlowski @ 2022-08-03 6:55 UTC (permalink / raw) To: Eddie James, joel Cc: linux, jdelvare, robh+dt, krzysztof.kozlowski+dt, linux-kernel, linux-hwmon, linux-fsi, devicetree On 02/08/2022 21:46, Eddie James wrote: > These bindings describe the POWER processor On Chip Controller accessed > from a service processor or baseboard management controller (BMC). > > Signed-off-by: Eddie James <eajames@linux.ibm.com> > --- > .../bindings/hwmon/ibm,occ-hmwon.yaml | 40 +++++++++++++++++++ > 1 file changed, 40 insertions(+) > create mode 100644 Documentation/devicetree/bindings/hwmon/ibm,occ-hmwon.yaml > > diff --git a/Documentation/devicetree/bindings/hwmon/ibm,occ-hmwon.yaml b/Documentation/devicetree/bindings/hwmon/ibm,occ-hmwon.yaml > new file mode 100644 > index 000000000000..8f8c3b8d7129 > --- /dev/null > +++ b/Documentation/devicetree/bindings/hwmon/ibm,occ-hmwon.yaml > @@ -0,0 +1,40 @@ > +# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause > +%YAML 1.2 > +--- > +$id: http://devicetree.org/schemas/hwmon/ibm,occ-hwmon.yaml# typo here Does not look like you tested the bindings. Please run `make dt_binding_check` (see Documentation/devicetree/bindings/writing-schema.rst for instructions). > +$schema: http://devicetree.org/meta-schemas/core.yaml# > + > +title: IBM On-Chip Controller (OCC) accessed from a service processor > + > +maintainers: > + - Eddie James <eajames@linux.ibm.com> > + > +description: | > + This binding describes a POWER processor On-Chip Controller (OCC) s/This binding describes a// But instead describe the hardware. What is the OCC? > + accessed from a service processor or baseboard management controller > + (BMC). > + > +properties: > + compatible: > + enum: > + - ibm,p9-occ-hwmon > + - ibm,p10-occ-hwmon > + > + ibm,inactive-on-init: > + description: This property describes whether or not the OCC should > + be marked as active during device initialization. The alternative > + is for user space to mark the device active based on higher level > + communications between the BMC and the host processor. I find the combination property name with this description confusing. It sounds like init of OCC and somehow it should be inactive? I assume if you initialize device, it is active. Or maybe the "init" is of something else? What is more, non-negation is easier to understand, so rather "ibm,active-on-boot" (or something like that). > + type: boolean > + > +required: > + - compatible > + > +additionalProperties: false > + > +examples: > + - | > + occ-hmwon { just "hwmon" > + compatible = "ibm,p9-occ-hwmon"; > + ibm,inactive-on-init; > + }; Best regards, Krzysztof ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 1/3] dt-bindings: hwmon: Add IBM OCC bindings 2022-08-03 6:55 ` Krzysztof Kozlowski @ 2022-08-09 19:42 ` Eddie James 2022-08-10 7:43 ` Krzysztof Kozlowski 0 siblings, 1 reply; 11+ messages in thread From: Eddie James @ 2022-08-09 19:42 UTC (permalink / raw) To: Krzysztof Kozlowski, joel Cc: linux, jdelvare, robh+dt, krzysztof.kozlowski+dt, linux-kernel, linux-hwmon, linux-fsi, devicetree On 8/3/22 01:55, Krzysztof Kozlowski wrote: > On 02/08/2022 21:46, Eddie James wrote: >> These bindings describe the POWER processor On Chip Controller accessed >> from a service processor or baseboard management controller (BMC). >> >> Signed-off-by: Eddie James <eajames@linux.ibm.com> >> --- >> .../bindings/hwmon/ibm,occ-hmwon.yaml | 40 +++++++++++++++++++ >> 1 file changed, 40 insertions(+) >> create mode 100644 Documentation/devicetree/bindings/hwmon/ibm,occ-hmwon.yaml >> >> diff --git a/Documentation/devicetree/bindings/hwmon/ibm,occ-hmwon.yaml b/Documentation/devicetree/bindings/hwmon/ibm,occ-hmwon.yaml >> new file mode 100644 >> index 000000000000..8f8c3b8d7129 >> --- /dev/null >> +++ b/Documentation/devicetree/bindings/hwmon/ibm,occ-hmwon.yaml >> @@ -0,0 +1,40 @@ >> +# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause >> +%YAML 1.2 >> +--- >> +$id: http://devicetree.org/schemas/hwmon/ibm,occ-hwmon.yaml# > typo here > > Does not look like you tested the bindings. Please run `make > dt_binding_check` (see > Documentation/devicetree/bindings/writing-schema.rst for instructions). I actually did but it didn't catch that somehow. I had to use a somewhat hacked together python/pip on my system so perhaps that's to blame. > >> +$schema: http://devicetree.org/meta-schemas/core.yaml# >> + >> +title: IBM On-Chip Controller (OCC) accessed from a service processor >> + >> +maintainers: >> + - Eddie James <eajames@linux.ibm.com> >> + >> +description: | >> + This binding describes a POWER processor On-Chip Controller (OCC) > s/This binding describes a// > But instead describe the hardware. What is the OCC? OK I'll fix that. It's a management engine for system power and thermals. > >> + accessed from a service processor or baseboard management controller >> + (BMC). >> + >> +properties: >> + compatible: >> + enum: >> + - ibm,p9-occ-hwmon >> + - ibm,p10-occ-hwmon >> + >> + ibm,inactive-on-init: >> + description: This property describes whether or not the OCC should >> + be marked as active during device initialization. The alternative >> + is for user space to mark the device active based on higher level >> + communications between the BMC and the host processor. > I find the combination property name with this description confusing. It > sounds like init of OCC and somehow it should be inactive? I assume if > you initialize device, it is active. Or maybe the "init" is of something > else? What is more, non-negation is easier to understand, so rather > "ibm,active-on-boot" (or something like that). Well, the host processor initializes the OCC during it's boot, but this document is describing a binding to be used by a service processor talking to the OCC. So the OCC may be in any state. The init meant driver init, but I will simply the description and change the property to be more explicit: ibm,no-poll-on-init since that is what is actually happening. Similar to the FSI binding for no-scan-on-init. > >> + type: boolean >> + >> +required: >> + - compatible >> + >> +additionalProperties: false >> + >> +examples: >> + - | >> + occ-hmwon { > just "hwmon" > >> + compatible = "ibm,p9-occ-hwmon"; >> + ibm,inactive-on-init; >> + }; Thanks for the review! Eddie > > Best regards, > Krzysztof ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 1/3] dt-bindings: hwmon: Add IBM OCC bindings 2022-08-09 19:42 ` Eddie James @ 2022-08-10 7:43 ` Krzysztof Kozlowski 2022-08-10 13:56 ` Eddie James 0 siblings, 1 reply; 11+ messages in thread From: Krzysztof Kozlowski @ 2022-08-10 7:43 UTC (permalink / raw) To: Eddie James, joel Cc: linux, jdelvare, robh+dt, krzysztof.kozlowski+dt, linux-kernel, linux-hwmon, linux-fsi, devicetree On 09/08/2022 22:42, Eddie James wrote: >>> + ibm,inactive-on-init: >>> + description: This property describes whether or not the OCC should >>> + be marked as active during device initialization. The alternative >>> + is for user space to mark the device active based on higher level >>> + communications between the BMC and the host processor. >> I find the combination property name with this description confusing. It >> sounds like init of OCC and somehow it should be inactive? I assume if >> you initialize device, it is active. Or maybe the "init" is of something >> else? What is more, non-negation is easier to understand, so rather >> "ibm,active-on-boot" (or something like that). > > > Well, the host processor initializes the OCC during it's boot, but this > document is describing a binding to be used by a service processor > talking to the OCC. So the OCC may be in any state. The init meant > driver init, but I will simply the description and change the property > to be more explicit: ibm,no-poll-on-init since that is what is actually > happening. Similar to the FSI binding for no-scan-on-init. no-scan-on-init is not a good example. It wasn't even reviewed by Rob (looking at commit). In both cases you describe driver behavior which is not appropriate for bindings. Instead you should describe the hardware characteristics/feature/bug/state which require skipping the initialization. Best regards, Krzysztof ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 1/3] dt-bindings: hwmon: Add IBM OCC bindings 2022-08-10 7:43 ` Krzysztof Kozlowski @ 2022-08-10 13:56 ` Eddie James 0 siblings, 0 replies; 11+ messages in thread From: Eddie James @ 2022-08-10 13:56 UTC (permalink / raw) To: Krzysztof Kozlowski, joel Cc: linux, jdelvare, robh+dt, krzysztof.kozlowski+dt, linux-kernel, linux-hwmon, linux-fsi, devicetree On 8/10/22 02:43, Krzysztof Kozlowski wrote: > On 09/08/2022 22:42, Eddie James wrote: >>>> + ibm,inactive-on-init: >>>> + description: This property describes whether or not the OCC should >>>> + be marked as active during device initialization. The alternative >>>> + is for user space to mark the device active based on higher level >>>> + communications between the BMC and the host processor. >>> I find the combination property name with this description confusing. It >>> sounds like init of OCC and somehow it should be inactive? I assume if >>> you initialize device, it is active. Or maybe the "init" is of something >>> else? What is more, non-negation is easier to understand, so rather >>> "ibm,active-on-boot" (or something like that). >> >> Well, the host processor initializes the OCC during it's boot, but this >> document is describing a binding to be used by a service processor >> talking to the OCC. So the OCC may be in any state. The init meant >> driver init, but I will simply the description and change the property >> to be more explicit: ibm,no-poll-on-init since that is what is actually >> happening. Similar to the FSI binding for no-scan-on-init. > no-scan-on-init is not a good example. It wasn't even reviewed by Rob > (looking at commit). In both cases you describe driver behavior which is > not appropriate for bindings. Instead you should describe the hardware > characteristics/feature/bug/state which require skipping the initialization. Yep... there is no such hardware thing. The driver should never poll the OCC during driver initialization (since host state isn't known), but it did for the first couple of years of the drivers existence because we didn't catch that it could cause problems. I submitted a patch to change the driver behavior, but it does change the user space interface, so the argument is that the new behavior should be optional. I suppose one correct way to control that would be a module parameter, but that really doesn't work well on embedded-like systems like ours. Thanks very much for your feedback Krzysztof. Joel, any suggestions here? Eddie > > > Best regards, > Krzysztof ^ permalink raw reply [flat|nested] 11+ messages in thread
* [PATCH 2/3] fsi: occ: Support probing the hwmon child device from dts node 2022-08-02 19:46 [PATCH 0/3] occ: Restore default behavior of polling OCC during init Eddie James 2022-08-02 19:46 ` [PATCH 1/3] dt-bindings: hwmon: Add IBM OCC bindings Eddie James @ 2022-08-02 19:46 ` Eddie James 2022-08-02 19:46 ` [PATCH 3/3] hwmon: (occ) Check for device property for setting OCC active during probe Eddie James 2 siblings, 0 replies; 11+ messages in thread From: Eddie James @ 2022-08-02 19:46 UTC (permalink / raw) To: joel Cc: linux, jdelvare, robh+dt, krzysztof.kozlowski+dt, linux-kernel, linux-hwmon, linux-fsi, devicetree, Eddie James There is now a need for reading devicetree properties in the OCC hwmon driver, which isn't current supported as the FSI driver just instantiates a basic platform device. Add support for this use case by checking for an "occ-hwmon" node and if present, creating an OF device from it. Signed-off-by: Eddie James <eajames@linux.ibm.com> --- drivers/fsi/fsi-occ.c | 41 ++++++++++++++++++++++++++++++++++------- 1 file changed, 34 insertions(+), 7 deletions(-) diff --git a/drivers/fsi/fsi-occ.c b/drivers/fsi/fsi-occ.c index 8f7f602b909d..abdd37d5507f 100644 --- a/drivers/fsi/fsi-occ.c +++ b/drivers/fsi/fsi-occ.c @@ -44,6 +44,7 @@ struct occ { struct device *sbefifo; char name[32]; int idx; + bool platform_hwmon; u8 sequence_number; void *buffer; void *client_buffer; @@ -598,7 +599,7 @@ int fsi_occ_submit(struct device *dev, const void *request, size_t req_len, } EXPORT_SYMBOL_GPL(fsi_occ_submit); -static int occ_unregister_child(struct device *dev, void *data) +static int occ_unregister_platform_child(struct device *dev, void *data) { struct platform_device *hwmon_dev = to_platform_device(dev); @@ -607,12 +608,25 @@ static int occ_unregister_child(struct device *dev, void *data) return 0; } +static int occ_unregister_of_child(struct device *dev, void *data) +{ + struct platform_device *hwmon_dev = to_platform_device(dev); + + of_device_unregister(hwmon_dev); + if (dev->of_node) + of_node_clear_flag(dev->of_node, OF_POPULATED); + + return 0; +} + static int occ_probe(struct platform_device *pdev) { int rc; u32 reg; + char child_name[32]; struct occ *occ; - struct platform_device *hwmon_dev; + struct platform_device *hwmon_dev = NULL; + struct device_node *hwmon_node; struct device *dev = &pdev->dev; struct platform_device_info hwmon_dev_info = { .parent = dev, @@ -671,10 +685,20 @@ static int occ_probe(struct platform_device *pdev) return rc; } - hwmon_dev_info.id = occ->idx; - hwmon_dev = platform_device_register_full(&hwmon_dev_info); - if (IS_ERR(hwmon_dev)) - dev_warn(dev, "failed to create hwmon device\n"); + hwmon_node = of_get_child_by_name(dev->of_node, hwmon_dev_info.name); + if (hwmon_node) { + snprintf(child_name, sizeof(child_name), "%s.%d", hwmon_dev_info.name, occ->idx); + hwmon_dev = of_platform_device_create(hwmon_node, child_name, dev); + of_node_put(hwmon_node); + } + + if (!hwmon_dev) { + occ->platform_hwmon = true; + hwmon_dev_info.id = occ->idx; + hwmon_dev = platform_device_register_full(&hwmon_dev_info); + if (IS_ERR(hwmon_dev)) + dev_warn(dev, "failed to create hwmon device\n"); + } return 0; } @@ -690,7 +714,10 @@ static int occ_remove(struct platform_device *pdev) occ->buffer = NULL; mutex_unlock(&occ->occ_lock); - device_for_each_child(&pdev->dev, NULL, occ_unregister_child); + if (occ->platform_hwmon) + device_for_each_child(&pdev->dev, NULL, occ_unregister_platform_child); + else + device_for_each_child(&pdev->dev, NULL, occ_unregister_of_child); ida_simple_remove(&occ_ida, occ->idx); -- 2.31.1 ^ permalink raw reply related [flat|nested] 11+ messages in thread
* [PATCH 3/3] hwmon: (occ) Check for device property for setting OCC active during probe 2022-08-02 19:46 [PATCH 0/3] occ: Restore default behavior of polling OCC during init Eddie James 2022-08-02 19:46 ` [PATCH 1/3] dt-bindings: hwmon: Add IBM OCC bindings Eddie James 2022-08-02 19:46 ` [PATCH 2/3] fsi: occ: Support probing the hwmon child device from dts node Eddie James @ 2022-08-02 19:46 ` Eddie James 2022-08-03 12:21 ` kernel test robot 2022-08-03 14:44 ` kernel test robot 2 siblings, 2 replies; 11+ messages in thread From: Eddie James @ 2022-08-02 19:46 UTC (permalink / raw) To: joel Cc: linux, jdelvare, robh+dt, krzysztof.kozlowski+dt, linux-kernel, linux-hwmon, linux-fsi, devicetree, Eddie James A previous commit changed the existing behavior of the driver to skip attempting to communicate with the OCC during probe. Return to the previous default behavior of automatically communicating with the OCC and make it optional with a new device-tree property. Signed-off-by: Eddie James <eajames@linux.ibm.com> --- drivers/hwmon/occ/common.c | 11 ++++++++++- drivers/hwmon/occ/p9_sbe.c | 9 +++++++++ 2 files changed, 19 insertions(+), 1 deletion(-) diff --git a/drivers/hwmon/occ/common.c b/drivers/hwmon/occ/common.c index 45407b12db4b..95f16bb0e685 100644 --- a/drivers/hwmon/occ/common.c +++ b/drivers/hwmon/occ/common.c @@ -10,6 +10,7 @@ #include <linux/math64.h> #include <linux/module.h> #include <linux/mutex.h> +#include <linux/property.h> #include <linux/sysfs.h> #include <asm/unaligned.h> @@ -1216,8 +1217,16 @@ int occ_setup(struct occ *occ) occ->groups[0] = &occ->group; rc = occ_setup_sysfs(occ); - if (rc) + if (rc) { dev_err(occ->bus_dev, "failed to setup sysfs: %d\n", rc); + return rc; + } + + if (!device_property_read_bool(occ->bus_dev, "ibm,inactive-on-init")) { + rc = occ_active(occ, true); + if (rc) + occ_shutdown_sysfs(occ); + } return rc; } diff --git a/drivers/hwmon/occ/p9_sbe.c b/drivers/hwmon/occ/p9_sbe.c index 4a1fe4ee8e2c..bc0f190065aa 100644 --- a/drivers/hwmon/occ/p9_sbe.c +++ b/drivers/hwmon/occ/p9_sbe.c @@ -7,6 +7,7 @@ #include <linux/fsi-occ.h> #include <linux/mm.h> #include <linux/module.h> +#include <linux/mod_devicetable.h> #include <linux/mutex.h> #include <linux/platform_device.h> #include <linux/string.h> @@ -179,9 +180,17 @@ static int p9_sbe_occ_remove(struct platform_device *pdev) return 0; } +static const struct of_device_id p9_sbe_occ_of_match[] = { + { .compatible = "ibm,p9-occ-hwmon" }, + { .compatible = "ibm,p10-occ-hwmon" }, + {} +}; +MODULE_DEVICE_TABLE(of, p8_i2c_occ_of_match); + static struct platform_driver p9_sbe_occ_driver = { .driver = { .name = "occ-hwmon", + .of_match_table = p9_sbe_occ_of_match, }, .probe = p9_sbe_occ_probe, .remove = p9_sbe_occ_remove, -- 2.31.1 ^ permalink raw reply related [flat|nested] 11+ messages in thread
* Re: [PATCH 3/3] hwmon: (occ) Check for device property for setting OCC active during probe 2022-08-02 19:46 ` [PATCH 3/3] hwmon: (occ) Check for device property for setting OCC active during probe Eddie James @ 2022-08-03 12:21 ` kernel test robot 2022-08-03 14:44 ` kernel test robot 1 sibling, 0 replies; 11+ messages in thread From: kernel test robot @ 2022-08-03 12:21 UTC (permalink / raw) To: Eddie James, joel Cc: llvm, kbuild-all, linux, jdelvare, robh+dt, krzysztof.kozlowski+dt, linux-kernel, linux-hwmon, linux-fsi, devicetree, Eddie James Hi Eddie, I love your patch! Yet something to improve: [auto build test ERROR on next-20220728] [also build test ERROR on v5.19] [cannot apply to groeck-staging/hwmon-next linus/master v5.19 v5.19-rc8 v5.19-rc7] [If your patch is applied to the wrong git tree, kindly drop us a note. And when submitting patch, we suggest to use '--base' as documented in https://git-scm.com/docs/git-format-patch#_base_tree_information] url: https://github.com/intel-lab-lkp/linux/commits/Eddie-James/occ-Restore-default-behavior-of-polling-OCC-during-init/20220803-034854 base: 7c5e07b73ff3011c9b82d4a3286a3362b951ad2b config: arm-randconfig-r021-20220803 (https://download.01.org/0day-ci/archive/20220803/202208032055.PEvfcqsc-lkp@intel.com/config) compiler: clang version 16.0.0 (https://github.com/llvm/llvm-project 495519e5f8232d144ed26e9c18dbcbac6a5f25eb) reproduce (this is a W=1 build): wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross chmod +x ~/bin/make.cross # install arm cross compiling tool for clang build # apt-get install binutils-arm-linux-gnueabi # https://github.com/intel-lab-lkp/linux/commit/31dc5bad51ddf22f4e097c0c5862d14341708188 git remote add linux-review https://github.com/intel-lab-lkp/linux git fetch --no-tags linux-review Eddie-James/occ-Restore-default-behavior-of-polling-OCC-during-init/20220803-034854 git checkout 31dc5bad51ddf22f4e097c0c5862d14341708188 # save the config file mkdir build_dir && cp config build_dir/.config COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross W=1 O=build_dir ARCH=arm SHELL=/bin/bash drivers/hwmon/occ/ If you fix the issue, kindly add following tag where applicable Reported-by: kernel test robot <lkp@intel.com> All errors (new ones prefixed by >>): >> drivers/hwmon/occ/p9_sbe.c:188:25: error: use of undeclared identifier 'p8_i2c_occ_of_match'; did you mean 'p9_sbe_occ_of_match'? MODULE_DEVICE_TABLE(of, p8_i2c_occ_of_match); ^~~~~~~~~~~~~~~~~~~ p9_sbe_occ_of_match include/linux/module.h:244:15: note: expanded from macro 'MODULE_DEVICE_TABLE' extern typeof(name) __mod_##type##__##name##_device_table \ ^ drivers/hwmon/occ/p9_sbe.c:183:34: note: 'p9_sbe_occ_of_match' declared here static const struct of_device_id p9_sbe_occ_of_match[] = { ^ 1 error generated. vim +188 drivers/hwmon/occ/p9_sbe.c 182 183 static const struct of_device_id p9_sbe_occ_of_match[] = { 184 { .compatible = "ibm,p9-occ-hwmon" }, 185 { .compatible = "ibm,p10-occ-hwmon" }, 186 {} 187 }; > 188 MODULE_DEVICE_TABLE(of, p8_i2c_occ_of_match); 189 -- 0-DAY CI Kernel Test Service https://01.org/lkp ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 3/3] hwmon: (occ) Check for device property for setting OCC active during probe 2022-08-02 19:46 ` [PATCH 3/3] hwmon: (occ) Check for device property for setting OCC active during probe Eddie James 2022-08-03 12:21 ` kernel test robot @ 2022-08-03 14:44 ` kernel test robot 1 sibling, 0 replies; 11+ messages in thread From: kernel test robot @ 2022-08-03 14:44 UTC (permalink / raw) To: Eddie James, joel Cc: kbuild-all, linux, jdelvare, robh+dt, krzysztof.kozlowski+dt, linux-kernel, linux-hwmon, linux-fsi, devicetree, Eddie James Hi Eddie, I love your patch! Yet something to improve: [auto build test ERROR on next-20220728] [also build test ERROR on v5.19] [cannot apply to groeck-staging/hwmon-next linus/master v5.19 v5.19-rc8 v5.19-rc7] [If your patch is applied to the wrong git tree, kindly drop us a note. And when submitting patch, we suggest to use '--base' as documented in https://git-scm.com/docs/git-format-patch#_base_tree_information] url: https://github.com/intel-lab-lkp/linux/commits/Eddie-James/occ-Restore-default-behavior-of-polling-OCC-during-init/20220803-034854 base: 7c5e07b73ff3011c9b82d4a3286a3362b951ad2b config: sh-allmodconfig (https://download.01.org/0day-ci/archive/20220803/202208032229.qba4UTzM-lkp@intel.com/config) compiler: sh4-linux-gcc (GCC) 12.1.0 reproduce (this is a W=1 build): wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross chmod +x ~/bin/make.cross # https://github.com/intel-lab-lkp/linux/commit/31dc5bad51ddf22f4e097c0c5862d14341708188 git remote add linux-review https://github.com/intel-lab-lkp/linux git fetch --no-tags linux-review Eddie-James/occ-Restore-default-behavior-of-polling-OCC-during-init/20220803-034854 git checkout 31dc5bad51ddf22f4e097c0c5862d14341708188 # save the config file mkdir build_dir && cp config build_dir/.config COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-12.1.0 make.cross W=1 O=build_dir ARCH=sh SHELL=/bin/bash drivers/hwmon/occ/ If you fix the issue, kindly add following tag where applicable Reported-by: kernel test robot <lkp@intel.com> All errors (new ones prefixed by >>): In file included from include/linux/device/driver.h:21, from include/linux/device.h:32, from drivers/hwmon/occ/p9_sbe.c:4: >> drivers/hwmon/occ/p9_sbe.c:188:25: error: 'p8_i2c_occ_of_match' undeclared here (not in a function); did you mean 'p9_sbe_occ_of_match'? 188 | MODULE_DEVICE_TABLE(of, p8_i2c_occ_of_match); | ^~~~~~~~~~~~~~~~~~~ include/linux/module.h:244:15: note: in definition of macro 'MODULE_DEVICE_TABLE' 244 | extern typeof(name) __mod_##type##__##name##_device_table \ | ^~~~ >> include/linux/module.h:244:21: error: '__mod_of__p8_i2c_occ_of_match_device_table' aliased to undefined symbol 'p8_i2c_occ_of_match' 244 | extern typeof(name) __mod_##type##__##name##_device_table \ | ^~~~~~ drivers/hwmon/occ/p9_sbe.c:188:1: note: in expansion of macro 'MODULE_DEVICE_TABLE' 188 | MODULE_DEVICE_TABLE(of, p8_i2c_occ_of_match); | ^~~~~~~~~~~~~~~~~~~ vim +188 drivers/hwmon/occ/p9_sbe.c 3 > 4 #include <linux/device.h> 5 #include <linux/errno.h> 6 #include <linux/slab.h> 7 #include <linux/fsi-occ.h> 8 #include <linux/mm.h> 9 #include <linux/module.h> 10 #include <linux/mod_devicetable.h> 11 #include <linux/mutex.h> 12 #include <linux/platform_device.h> 13 #include <linux/string.h> 14 #include <linux/sysfs.h> 15 16 #include "common.h" 17 18 #define OCC_CHECKSUM_RETRIES 3 19 20 struct p9_sbe_occ { 21 struct occ occ; 22 bool sbe_error; 23 void *ffdc; 24 size_t ffdc_len; 25 size_t ffdc_size; 26 struct mutex sbe_error_lock; /* lock access to ffdc data */ 27 struct device *sbe; 28 }; 29 30 #define to_p9_sbe_occ(x) container_of((x), struct p9_sbe_occ, occ) 31 32 static ssize_t ffdc_read(struct file *filp, struct kobject *kobj, 33 struct bin_attribute *battr, char *buf, loff_t pos, 34 size_t count) 35 { 36 ssize_t rc = 0; 37 struct occ *occ = dev_get_drvdata(kobj_to_dev(kobj)); 38 struct p9_sbe_occ *ctx = to_p9_sbe_occ(occ); 39 40 mutex_lock(&ctx->sbe_error_lock); 41 if (ctx->sbe_error) { 42 rc = memory_read_from_buffer(buf, count, &pos, ctx->ffdc, 43 ctx->ffdc_len); 44 if (pos >= ctx->ffdc_len) 45 ctx->sbe_error = false; 46 } 47 mutex_unlock(&ctx->sbe_error_lock); 48 49 return rc; 50 } 51 static BIN_ATTR_RO(ffdc, OCC_MAX_RESP_WORDS * 4); 52 53 static bool p9_sbe_occ_save_ffdc(struct p9_sbe_occ *ctx, const void *resp, 54 size_t resp_len) 55 { 56 bool notify = false; 57 58 mutex_lock(&ctx->sbe_error_lock); 59 if (!ctx->sbe_error) { 60 if (resp_len > ctx->ffdc_size) { 61 kvfree(ctx->ffdc); 62 ctx->ffdc = kvmalloc(resp_len, GFP_KERNEL); 63 if (!ctx->ffdc) { 64 ctx->ffdc_len = 0; 65 ctx->ffdc_size = 0; 66 goto done; 67 } 68 69 ctx->ffdc_size = resp_len; 70 } 71 72 notify = true; 73 ctx->sbe_error = true; 74 ctx->ffdc_len = resp_len; 75 memcpy(ctx->ffdc, resp, resp_len); 76 } 77 78 done: 79 mutex_unlock(&ctx->sbe_error_lock); 80 return notify; 81 } 82 83 static int p9_sbe_occ_send_cmd(struct occ *occ, u8 *cmd, size_t len, 84 void *resp, size_t resp_len) 85 { 86 struct p9_sbe_occ *ctx = to_p9_sbe_occ(occ); 87 int rc, i; 88 89 for (i = 0; i < OCC_CHECKSUM_RETRIES; ++i) { 90 rc = fsi_occ_submit(ctx->sbe, cmd, len, resp, &resp_len); 91 if (rc >= 0) 92 break; 93 if (resp_len) { 94 if (p9_sbe_occ_save_ffdc(ctx, resp, resp_len)) 95 sysfs_notify(&occ->bus_dev->kobj, NULL, 96 bin_attr_ffdc.attr.name); 97 return rc; 98 } 99 if (rc != -EBADE) 100 return rc; 101 } 102 103 switch (((struct occ_response *)resp)->return_status) { 104 case OCC_RESP_CMD_IN_PRG: 105 rc = -ETIMEDOUT; 106 break; 107 case OCC_RESP_SUCCESS: 108 rc = 0; 109 break; 110 case OCC_RESP_CMD_INVAL: 111 case OCC_RESP_CMD_LEN_INVAL: 112 case OCC_RESP_DATA_INVAL: 113 case OCC_RESP_CHKSUM_ERR: 114 rc = -EINVAL; 115 break; 116 case OCC_RESP_INT_ERR: 117 case OCC_RESP_BAD_STATE: 118 case OCC_RESP_CRIT_EXCEPT: 119 case OCC_RESP_CRIT_INIT: 120 case OCC_RESP_CRIT_WATCHDOG: 121 case OCC_RESP_CRIT_OCB: 122 case OCC_RESP_CRIT_HW: 123 rc = -EREMOTEIO; 124 break; 125 default: 126 rc = -EPROTO; 127 } 128 129 return rc; 130 } 131 132 static int p9_sbe_occ_probe(struct platform_device *pdev) 133 { 134 int rc; 135 struct occ *occ; 136 struct p9_sbe_occ *ctx = devm_kzalloc(&pdev->dev, sizeof(*ctx), 137 GFP_KERNEL); 138 if (!ctx) 139 return -ENOMEM; 140 141 mutex_init(&ctx->sbe_error_lock); 142 143 ctx->sbe = pdev->dev.parent; 144 occ = &ctx->occ; 145 occ->bus_dev = &pdev->dev; 146 platform_set_drvdata(pdev, occ); 147 148 occ->powr_sample_time_us = 500; 149 occ->poll_cmd_data = 0x20; /* P9 OCC poll data */ 150 occ->send_cmd = p9_sbe_occ_send_cmd; 151 152 rc = occ_setup(occ); 153 if (rc == -ESHUTDOWN) 154 rc = -ENODEV; /* Host is shutdown, don't spew errors */ 155 156 if (!rc) { 157 rc = device_create_bin_file(occ->bus_dev, &bin_attr_ffdc); 158 if (rc) { 159 dev_warn(occ->bus_dev, 160 "failed to create SBE error ffdc file\n"); 161 rc = 0; 162 } 163 } 164 165 return rc; 166 } 167 168 static int p9_sbe_occ_remove(struct platform_device *pdev) 169 { 170 struct occ *occ = platform_get_drvdata(pdev); 171 struct p9_sbe_occ *ctx = to_p9_sbe_occ(occ); 172 173 device_remove_bin_file(occ->bus_dev, &bin_attr_ffdc); 174 175 ctx->sbe = NULL; 176 occ_shutdown(occ); 177 178 kvfree(ctx->ffdc); 179 180 return 0; 181 } 182 183 static const struct of_device_id p9_sbe_occ_of_match[] = { 184 { .compatible = "ibm,p9-occ-hwmon" }, 185 { .compatible = "ibm,p10-occ-hwmon" }, 186 {} 187 }; > 188 MODULE_DEVICE_TABLE(of, p8_i2c_occ_of_match); 189 -- 0-DAY CI Kernel Test Service https://01.org/lkp ^ permalink raw reply [flat|nested] 11+ messages in thread
end of thread, other threads:[~2022-08-10 13:57 UTC | newest] Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2022-08-02 19:46 [PATCH 0/3] occ: Restore default behavior of polling OCC during init Eddie James 2022-08-02 19:46 ` [PATCH 1/3] dt-bindings: hwmon: Add IBM OCC bindings Eddie James 2022-08-02 22:38 ` Rob Herring 2022-08-03 6:55 ` Krzysztof Kozlowski 2022-08-09 19:42 ` Eddie James 2022-08-10 7:43 ` Krzysztof Kozlowski 2022-08-10 13:56 ` Eddie James 2022-08-02 19:46 ` [PATCH 2/3] fsi: occ: Support probing the hwmon child device from dts node Eddie James 2022-08-02 19:46 ` [PATCH 3/3] hwmon: (occ) Check for device property for setting OCC active during probe Eddie James 2022-08-03 12:21 ` kernel test robot 2022-08-03 14:44 ` kernel test robot
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).