在 2023/4/25 14:08, Arnd Bergmann 写道:
On Tue, Apr 25, 2023, at 05:04, lihuisong (C) wrote:
在 2023/4/24 16:09, Arnd Bergmann 写道:
On Mon, Apr 24, 2023, at 09:30, Huisong Li wrote:
depends on ACPI
depends on (ARM64 && ARCH_HISI) || COMPILE_TEST
What do you think of adjusting it as below?
menu "Hisilicon SoC drivers"
depends on ARCH_HISI || COMPILE_TEST
config KUNPENG_HCCS
depends on ACPI
depends on ARM64 || COMPILE_TEST
Yes, that's perfect.
+
+#include "kunpeng_hccs.h"
+
+/* PCC defines */
+#define HCCS_PCC_SIGNATURE_MASK 0x50434300
+#define HCCS_PCC_STATUS_CMD_COMPLETE BIT(0)
Should these perhaps be in include/acpi/pcc.h? The 0x50434300
number is just "PCC\0", so it appears to not be HCCS specific.
This is a PCC signature. As stated in the APCI,
"The signature of a subspace is computed by a bitwiseor of the value
0x50434300
with the subspace ID. For example, subspace 3 has the signature 0x50434303."
I am not sure if all driver need to use this fixed signature mask.
As far as I know, cppc_acpi.c didn't use this signature and
xgene-hwmon.c used another mask defined in its driver.
So I place it here.
I would still put it into the generic header, but it doesn't
really matter much, so do it whichever way you prefer. No need
for a separate patch if you decide to use the global header,
it can just be part of your normal patch.
ok, keep it the way it is now.
+
+static int hccs_get_device_property(struct hccs_dev *hdev)
+{
+ struct device *dev = hdev->dev;
+
+ if (device_property_read_u32(dev, "device-flags", &hdev->flags)) {
+ dev_err(hdev->dev, "no device-flags property.\n");
+ return -ENODEV;
+ }
+
+ if (device_property_read_u8(dev, "pcc-type", &hdev->type)) {
+ dev_err(hdev->dev, "no pcc-type property.\n");
+ return -ENODEV;
+ }
+
+ if (device_property_read_u32(dev, "pcc-chan-id", &hdev->chan_id)) {
+ dev_err(hdev->dev, "no pcc-channel property.\n");
+ return -ENODEV;
+ }
+
+ hdev->intr_mode = hccs_get_bit(hdev->flags, HCCS_DEV_FLAGS_INTR_B);
+ if (!hccs_dev_property_supported(hdev))
+ return -EOPNOTSUPP;
+
Where are the device properties documented? I'm never quite sure how
to handle these for ACPI-only drivers, since we don't normally have the
bindings in Documentation/devicetree/bindings/, but it feels like there
should be some properly reviewed document somewhere else.
These are ACPI-only, instead of DT.
I will add a comment here as Krzysztof suggested.
I understand that they are ACPI-only, what I'm more interested here is
the general question of how we should document them, to ensure these
are handled consistently across drivers.
These device properties are reported by ACPI table in firmware.
They are fixed in platform firmware. The user cannot modify them.
Do we need to document them?
--- /dev/null
+++ b/drivers/soc/hisilicon/kunpeng_hccs.h
@@ -0,0 +1,204 @@
+/* SPDX-License-Identifier: GPL-2.0+ */
+/* Copyright (c) 2023 Hisilicon Limited. */
+
+#ifndef __KUNPENG_HCCS_H__
+#define __KUNPENG_HCCS_H__
Are you planning to add more drivers that share this file? If not,
just fold the contents into the driver itself.
Yes, we will add more drivers in this file.
Ok.
Arnd
.
