* Re: [PATCH 1/6] dt-bindings: adc: ad7173: add support for ad411x
From: Ceclan, Dumitru @ 2024-04-03 7:43 UTC (permalink / raw)
To: David Lechner, dumitru.ceclan
Cc: Lars-Peter Clausen, Michael Hennerich, Jonathan Cameron,
Rob Herring, Krzysztof Kozlowski, Conor Dooley, linux-iio,
devicetree, linux-kernel
In-Reply-To: <CAMknhBHeKAQ45=5-dL1T1tv-mZcPN+bNo3vxWJYgWpEPE+8p3Q@mail.gmail.com>
On 01/04/2024 22:37, David Lechner wrote:
> On Mon, Apr 1, 2024 at 10:10 AM Dumitru Ceclan via B4 Relay
> <devnull+dumitru.ceclan.analog.com@kernel.org> wrote:
>>
>> From: Dumitru Ceclan <dumitru.ceclan@analog.com>
...
>> properties:
>> reg:
>> + description:
>> + Reg values 16-19 are only permitted for ad4111/ad4112 current channels.
>> minimum: 0
>> - maximum: 15
>> + maximum: 19
>
> This looks wrong. Isn't reg describing the number of logical channels
> (# of channel config registers)?
>
> After reviewing the driver, I see that > 16 is used as a way of
> flagging current inputs, but still seems like the wrong way to do it.
> See suggestion below.
>
This was a suggestion from Jonathan, maybe I implemented it wrong.
Other alternative that came to my mind: attribute "adi,current-channel".
>>
>> diff-channels:
>> + description:
>> + For using current channels specify only the positive channel.
>> + (IIN2+, IIN2−) -> diff-channels = <2 0>
>
> I find this a bit confusing since 2 is already VIN2 and 0 is already
> VIN0. I think it would make more sense to assign unique channel
> numbers individually to the negative and positive current inputs.
> Also, I think it makes sense to use the same numbers that the
> registers in the datasheet use (8 - 11 for negative and 12 to 15 for
> positive).
>
> So: (IIN2+, IIN2−) -> diff-channels = <13 10>
>
>
It would mean for the user to look in the datasheet at the possible
channel INPUT configurations values, decode the bit field into two
integer values and place it here (0110101010) -> 13 10. This is
cumbersome for just choosing current input 2.
>> +
>> + Family AD411x supports a dedicated VCOM voltage input.
>> + To select it set the second channel to 16.
>> + (VIN2, VCOM) -> diff-channels = <2 16>
>
> The 411x datasheets call this pin VINCOM so calling it VCOM here is a
> bit confusing.
>
Sure, I'll rename to VINCOM.
> Also, do we need to add a vincom-supply to get this voltage? Or is it
> safe to assume it is always connected to AVSS? The datasheet seems to
> indicate that the latter is the case. But then it also has this
> special case (at least for AD4116, didn't check all datasheets)
> "VIN10, VINCOM (single-ended or differential pair)". If it can be used
> as part of a fully differential input, we probably need some extra
> flag to indicate that case.
>
I cannot see any configuration options for these use cases. All inputs
are routed to the same mux and routed to the differential positive and
negative ADC inputs.
"VIN10, VINCOM (single-ended or differential pair)" the only difference
between these two use cases is if you connected VINCOM to AVSS (with
unipolar coding) or not with bipolar encoding. The channel is still
measuring the difference between the two selected inputs and comparing
to the selected reference.
> Similarly, do we need special handling for ADCIN15 on AD4116? It has a
> "(pseudo differential or differential pair)" notation that other
> inputs don't. In other words, it is more like VINCOM than it is to the
> other ADCINxx pins. So we probably need an adcin15-supply for this pin
> to properly get the right channel configuration. I.e. the logic in the
> IIO driver would be if adcin15-supply is present, any channels that
> use this input are pseudo-differential, otherwise any channels that
> use it are fully differential.
>
I cannot seem to understand what would a adcin15-supply be needed for.
This input, the same as all others, enters the mux and is routed to
either positive or negative input of the ADC.
The voltage on the ADCIN15 pin is not important to the user, just the
difference in voltage between that pin and the other one selected.
>> items:
>> minimum: 0
>> maximum: 31
>> @@ -166,7 +191,6 @@ allOf:
>> - $ref: /schemas/spi/spi-peripheral-props.yaml#
>>
>> # Only ad7172-4, ad7173-8 and ad7175-8 support vref2
>> - # Other models have [0-3] channel registers
>
> Did you forget to remove
>
> reg:
> maximum: 3
>
> from this if statement that this comment is referring to?
>
>
Other way around, forgot in a previous patch to remove the comment.
I'll move this change to a precursor patch.
>> - if:
>> properties:
>> compatible:
>> @@ -187,6 +211,37 @@ allOf:
>> - vref
>> - refout-avss
>> - avdd
>> +
>> + - if:
>> + properties:
>> + compatible:
>> + contains:
>> + enum:
>> + - adi,ad4114
>> + - adi,ad4115
>> + - adi,ad4116
>> + - adi,ad7173-8
>> + - adi,ad7175-8
>> + then:
>> + patternProperties:
>> + "^channel@[0-9a-f]$":
>> + properties:
>> + reg:
>> + maximum: 15
>
> As with the previous reg comment, this if statement should not be
> needed since maximum should not be changed to 19.
>
We'll see what is the best approach regarding the current channels,
perhaps the one you mentioned in the later reply with always configuring
like the temp channel.
>> +
>> + - if:
>> + properties:
>> + compatible:
>> + contains:
>> + enum:
>> + - adi,ad7172-2
>> + - adi,ad7175-2
>> + - adi,ad7176-2
>> + - adi,ad7177-2
>> + then:
>> + patternProperties:
>> + "^channel@[0-9a-f]$":
>> + properties:
>> reg:
>> maximum: 3
>
> It looks to me like AD7172-4 actually has 8 possible channels rather
> than 16. So it would need a special condition as well. But that is a
> bug in the previous bindings and should therefore be fixed in a
> separate patch.
It is addressed already in the binding:
"
- if:
properties:
compatible:
contains:
const: adi,ad7172-4
[...]
maximum: 7
"
^ permalink raw reply
* Re: [PATCH v18 2/9] usb: dwc3: core: Access XHCI address space temporarily to read port info
From: Johan Hovold @ 2024-04-03 7:40 UTC (permalink / raw)
To: Krishna Kurapati PSSNV
Cc: Thinh Nguyen, Krzysztof Kozlowski, Rob Herring, Bjorn Andersson,
Wesley Cheng, Konrad Dybcio, Greg Kroah-Hartman, Conor Dooley,
Felipe Balbi, devicetree@vger.kernel.org,
linux-arm-msm@vger.kernel.org, linux-usb@vger.kernel.org,
linux-kernel@vger.kernel.org, quic_ppratap@quicinc.com,
quic_jackp@quicinc.com, Johan Hovold
In-Reply-To: <c30811e6-2a1e-4b26-8b94-7c67000b8568@quicinc.com>
On Wed, Apr 03, 2024 at 01:03:21PM +0530, Krishna Kurapati PSSNV wrote:
> On 4/3/2024 12:36 PM, Johan Hovold wrote:
> > The series has not been merged yet so you can address both issues in a
> > v19. Perhaps wait a day or two in case Thinh has further comments.
> Also after making the following two changes:
>
> 1. Rename dwc3_read_port_info(...) to dwc3_get_num_ports(...)
> 2. Changing "if (IS_ERR(base))" to "if (!base)"
>
> Can I still retain your RB tag ?
Yes, please do.
Johan
^ permalink raw reply
* Re: [PATCH v8 7/7] spmi: pmic-arb: Add multi bus support
From: Neil Armstrong @ 2024-04-03 7:37 UTC (permalink / raw)
To: Abel Vesa, Stephen Boyd, Matthias Brugger, Bjorn Andersson,
Konrad Dybcio, Dmitry Baryshkov, AngeloGioacchino Del Regno,
Rob Herring, Krzysztof Kozlowski, Conor Dooley
Cc: Srini Kandagatla, Johan Hovold, linux-kernel, linux-arm-kernel,
linux-arm-msm, linux-mediatek, devicetree
In-Reply-To: <20240402-spmi-multi-master-support-v8-7-ce6f2d14a058@linaro.org>
On 02/04/2024 14:07, Abel Vesa wrote:
> Starting with HW version 7, there are actually two separate buses
> (with two separate sets of wires). So add support for the second bus.
> The first platform that needs this support for the second bus is the
> Qualcomm X1 Elite, so add the compatible for it as well.
>
> Signed-off-by: Abel Vesa <abel.vesa@linaro.org>
> ---
> drivers/spmi/spmi-pmic-arb.c | 138 +++++++++++++++++++++++++++++++++++++------
> 1 file changed, 120 insertions(+), 18 deletions(-)
>
> diff --git a/drivers/spmi/spmi-pmic-arb.c b/drivers/spmi/spmi-pmic-arb.c
> index 3db622ed80de..52b9e275a7b2 100644
> --- a/drivers/spmi/spmi-pmic-arb.c
> +++ b/drivers/spmi/spmi-pmic-arb.c
> @@ -13,6 +13,7 @@
> #include <linux/kernel.h>
> #include <linux/module.h>
> #include <linux/of.h>
> +#include <linux/of_address.h>
> #include <linux/of_irq.h>
> #include <linux/platform_device.h>
> #include <linux/slab.h>
> @@ -95,6 +96,8 @@ enum pmic_arb_channel {
> PMIC_ARB_CHANNEL_OBS,
> };
>
> +#define PMIC_ARB_MAX_BUSES 2
> +
> /* Maximum number of support PMIC peripherals */
> #define PMIC_ARB_MAX_PERIPHS 512
> #define PMIC_ARB_MAX_PERIPHS_V7 1024
> @@ -148,6 +151,7 @@ struct spmi_pmic_arb;
> * @min_apid: minimum APID (used for bounding IRQ search)
> * @max_apid: maximum APID
> * @irq: PMIC ARB interrupt.
> + * @id: unique ID of the bus
> */
> struct spmi_pmic_arb_bus {
> struct spmi_pmic_arb *pmic_arb;
> @@ -165,6 +169,7 @@ struct spmi_pmic_arb_bus {
> u16 min_apid;
> u16 max_apid;
> int irq;
> + u8 id;
> };
>
> /**
> @@ -179,7 +184,8 @@ struct spmi_pmic_arb_bus {
> * @ee: the current Execution Environment
> * @ver_ops: version dependent operations.
> * @max_periphs: Number of elements in apid_data[]
> - * @bus: per arbiter bus instance
> + * @buses: per arbiter buses instances
> + * @buses_available: number of buses registered
> */
> struct spmi_pmic_arb {
> void __iomem *rd_base;
> @@ -191,7 +197,8 @@ struct spmi_pmic_arb {
> u8 ee;
> const struct pmic_arb_ver_ops *ver_ops;
> int max_periphs;
> - struct spmi_pmic_arb_bus *bus;
> + struct spmi_pmic_arb_bus *buses[PMIC_ARB_MAX_BUSES];
> + int buses_available;
> };
>
> /**
> @@ -220,7 +227,7 @@ struct spmi_pmic_arb {
> struct pmic_arb_ver_ops {
> const char *ver_str;
> int (*get_core_resources)(struct platform_device *pdev, void __iomem *core);
> - int (*init_apid)(struct spmi_pmic_arb_bus *bus);
> + int (*init_apid)(struct spmi_pmic_arb_bus *bus, int index);
> int (*ppid_to_apid)(struct spmi_pmic_arb_bus *bus, u16 ppid);
> /* spmi commands (read_cmd, write_cmd, cmd) functionality */
> int (*offset)(struct spmi_pmic_arb_bus *bus, u8 sid, u16 addr,
> @@ -309,8 +316,8 @@ static int pmic_arb_wait_for_done(struct spmi_controller *ctrl,
> }
>
> if (status & PMIC_ARB_STATUS_FAILURE) {
> - dev_err(&ctrl->dev, "%s: %#x %#x: transaction failed (%#x)\n",
> - __func__, sid, addr, status);
> + dev_err(&ctrl->dev, "%s: %#x %#x: transaction failed (%#x) reg: 0x%x\n",
> + __func__, sid, addr, status, offset);
> WARN_ON(1);
> return -EIO;
> }
> @@ -326,8 +333,8 @@ static int pmic_arb_wait_for_done(struct spmi_controller *ctrl,
> udelay(1);
> }
>
> - dev_err(&ctrl->dev, "%s: %#x %#x: timeout, status %#x\n",
> - __func__, sid, addr, status);
> + dev_err(&ctrl->dev, "%s: %#x %#x %#x: timeout, status %#x\n",
> + __func__, bus->id, sid, addr, status);
> return -ETIMEDOUT;
> }
>
> @@ -1006,11 +1013,17 @@ static int pmic_arb_get_core_resources_v1(struct platform_device *pdev,
> return 0;
> }
>
> -static int pmic_arb_init_apid_v1(struct spmi_pmic_arb_bus *bus)
> +static int pmic_arb_init_apid_v1(struct spmi_pmic_arb_bus *bus, int index)
> {
> struct spmi_pmic_arb *pmic_arb = bus->pmic_arb;
> u32 *mapping_table;
>
> + if (index) {
> + dev_err(&bus->spmic->dev, "Unsupported buses count %d detected\n",
> + index);
> + return -EINVAL;
> + }
> +
> mapping_table = devm_kcalloc(&bus->spmic->dev, pmic_arb->max_periphs,
> sizeof(*mapping_table), GFP_KERNEL);
> if (!mapping_table)
> @@ -1253,11 +1266,17 @@ static int pmic_arb_offset_v2(struct spmi_pmic_arb_bus *bus, u8 sid, u16 addr,
> return 0x1000 * pmic_arb->ee + 0x8000 * apid;
> }
>
> -static int pmic_arb_init_apid_v5(struct spmi_pmic_arb_bus *bus)
> +static int pmic_arb_init_apid_v5(struct spmi_pmic_arb_bus *bus, int index)
> {
> struct spmi_pmic_arb *pmic_arb = bus->pmic_arb;
> int ret;
>
> + if (index) {
> + dev_err(&bus->spmic->dev, "Unsupported buses count %d detected\n",
> + index);
> + return -EINVAL;
> + }
> +
> bus->base_apid = 0;
> bus->apid_count = readl_relaxed(pmic_arb->core + PMIC_ARB_FEATURES) &
> PMIC_ARB_FEATURES_PERIPH_MASK;
> @@ -1329,6 +1348,50 @@ static int pmic_arb_get_core_resources_v7(struct platform_device *pdev,
> return pmic_arb_get_obsrvr_chnls_v2(pdev);
> }
>
> +/*
> + * Only v7 supports 2 buses. Each bus will get a different apid count, read
> + * from different registers.
> + */
> +static int pmic_arb_init_apid_v7(struct spmi_pmic_arb_bus *bus, int index)
> +{
> + struct spmi_pmic_arb *pmic_arb = bus->pmic_arb;
> + int ret;
> +
> + if (index == 0) {
> + bus->base_apid = 0;
> + bus->apid_count = readl_relaxed(pmic_arb->core + PMIC_ARB_FEATURES) &
> + PMIC_ARB_FEATURES_PERIPH_MASK;
> + } else if (index == 1) {
> + bus->base_apid = readl_relaxed(pmic_arb->core + PMIC_ARB_FEATURES) &
> + PMIC_ARB_FEATURES_PERIPH_MASK;
> + bus->apid_count = readl_relaxed(pmic_arb->core + PMIC_ARB_FEATURES1) &
> + PMIC_ARB_FEATURES_PERIPH_MASK;
> + } else {
> + dev_err(&bus->spmic->dev, "Unsupported buses count %d detected\n",
> + bus->id);
> + return -EINVAL;
> + }
> +
> + if (bus->base_apid + bus->apid_count > pmic_arb->max_periphs) {
> + dev_err(&bus->spmic->dev, "Unsupported APID count %d detected\n",
> + bus->base_apid + bus->apid_count);
> + return -EINVAL;
> + }
> +
> + ret = pmic_arb_init_apid_min_max(bus);
> + if (ret)
> + return ret;
> +
> + ret = pmic_arb_read_apid_map_v5(bus);
> + if (ret) {
> + dev_err(&bus->spmic->dev, "could not read APID->PPID mapping table, rc= %d\n",
> + ret);
> + return ret;
> + }
> +
> + return 0;
> +}
> +
> /*
> * v7 offset per ee and per apid for observer channels and per apid for
> * read/write channels.
> @@ -1581,7 +1644,7 @@ static const struct pmic_arb_ver_ops pmic_arb_v5 = {
> static const struct pmic_arb_ver_ops pmic_arb_v7 = {
> .ver_str = "v7",
> .get_core_resources = pmic_arb_get_core_resources_v7,
> - .init_apid = pmic_arb_init_apid_v5,
> + .init_apid = pmic_arb_init_apid_v7,
> .ppid_to_apid = pmic_arb_ppid_to_apid_v5,
> .non_data_cmd = pmic_arb_non_data_cmd_v2,
> .offset = pmic_arb_offset_v7,
> @@ -1605,6 +1668,7 @@ static int spmi_pmic_arb_bus_init(struct platform_device *pdev,
> struct device_node *node,
> struct spmi_pmic_arb *pmic_arb)
> {
> + int bus_index = pmic_arb->buses_available;
> struct spmi_pmic_arb_bus *bus;
> struct device *dev = &pdev->dev;
> struct spmi_controller *ctrl;
> @@ -1623,7 +1687,7 @@ static int spmi_pmic_arb_bus_init(struct platform_device *pdev,
>
> bus = spmi_controller_get_drvdata(ctrl);
>
> - pmic_arb->bus = bus;
> + pmic_arb->buses[bus_index] = bus;
>
> bus->ppid_to_apid = devm_kcalloc(dev, PMIC_ARB_MAX_PPID,
> sizeof(*bus->ppid_to_apid),
> @@ -1666,12 +1730,13 @@ static int spmi_pmic_arb_bus_init(struct platform_device *pdev,
> bus->cnfg = cnfg;
> bus->irq = irq;
> bus->spmic = ctrl;
> + bus->id = bus_index;
>
> - ret = pmic_arb->ver_ops->init_apid(bus);
> + ret = pmic_arb->ver_ops->init_apid(bus, bus_index);
> if (ret)
> return ret;
>
> - dev_dbg(&pdev->dev, "adding irq domain\n");
> + dev_dbg(&pdev->dev, "adding irq domain for bus %d\n", bus_index);
>
> bus->domain = irq_domain_add_tree(dev->of_node,
> &pmic_arb_irq_domain_ops, bus);
> @@ -1684,14 +1749,53 @@ static int spmi_pmic_arb_bus_init(struct platform_device *pdev,
> pmic_arb_chained_irq, bus);
>
> ctrl->dev.of_node = node;
> + dev_set_name(&ctrl->dev, "spmi-%d", bus_index);
>
> ret = devm_spmi_controller_add(dev, ctrl);
> if (ret)
> return ret;
>
> + pmic_arb->buses_available++;
> +
> return 0;
> }
>
> +static int spmi_pmic_arb_register_buses(struct spmi_pmic_arb *pmic_arb,
> + struct platform_device *pdev)
> +{
> + struct device *dev = &pdev->dev;
> + struct device_node *node = dev->of_node;
> + struct device_node *child;
> + int ret;
> +
> + /* legacy mode doesn't provide child node for the bus */
> + if (of_device_is_compatible(node, "qcom,spmi-pmic-arb"))
> + return spmi_pmic_arb_bus_init(pdev, node, pmic_arb);
> +
> + for_each_available_child_of_node(node, child) {
> + if (of_node_name_eq(child, "spmi")) {
> + ret = spmi_pmic_arb_bus_init(pdev, child, pmic_arb);
> + if (ret)
> + return ret;
> + }
> + }
> +
> + return ret;
> +}
> +
> +static void spmi_pmic_arb_deregister_buses(struct spmi_pmic_arb *pmic_arb)
> +{
> + int i;
> +
> + for (i = 0; i < PMIC_ARB_MAX_BUSES; i++) {
> + struct spmi_pmic_arb_bus *bus = pmic_arb->buses[i];
> +
> + irq_set_chained_handler_and_data(bus->irq,
> + NULL, NULL);
> + irq_domain_remove(bus->domain);
> + }
> +}
> +
> static int spmi_pmic_arb_probe(struct platform_device *pdev)
> {
> struct spmi_pmic_arb *pmic_arb;
> @@ -1762,21 +1866,19 @@ static int spmi_pmic_arb_probe(struct platform_device *pdev)
>
> pmic_arb->ee = ee;
>
> - return spmi_pmic_arb_bus_init(pdev, dev->of_node, pmic_arb);
> + return spmi_pmic_arb_register_buses(pmic_arb, pdev);
> }
>
> static void spmi_pmic_arb_remove(struct platform_device *pdev)
> {
> struct spmi_pmic_arb *pmic_arb = platform_get_drvdata(pdev);
> - struct spmi_pmic_arb_bus *bus = pmic_arb->bus;
>
> - irq_set_chained_handler_and_data(bus->irq,
> - NULL, NULL);
> - irq_domain_remove(bus->domain);
> + spmi_pmic_arb_deregister_buses(pmic_arb);
> }
>
> static const struct of_device_id spmi_pmic_arb_match_table[] = {
> { .compatible = "qcom,spmi-pmic-arb", },
> + { .compatible = "qcom,x1e80100-spmi-pmic-arb", },
> {},
> };
> MODULE_DEVICE_TABLE(of, spmi_pmic_arb_match_table);
>
Reviewed-by: Neil Armstrong <neil.armstrong@linaro.org>
^ permalink raw reply
* Re: [PATCH v8 6/7] spmi: pmic-arb: Register controller for bus instead of arbiter
From: Neil Armstrong @ 2024-04-03 7:37 UTC (permalink / raw)
To: Abel Vesa, Stephen Boyd, Matthias Brugger, Bjorn Andersson,
Konrad Dybcio, Dmitry Baryshkov, AngeloGioacchino Del Regno,
Rob Herring, Krzysztof Kozlowski, Conor Dooley
Cc: Srini Kandagatla, Johan Hovold, linux-kernel, linux-arm-kernel,
linux-arm-msm, linux-mediatek, devicetree
In-Reply-To: <20240402-spmi-multi-master-support-v8-6-ce6f2d14a058@linaro.org>
On 02/04/2024 14:07, Abel Vesa wrote:
> Introduce the bus object in order to decouple the resources
> that are bus specific from the arbiter. This way the SPMI controller
> is registered with the generic framework at a bus level rather than
> arbiter. This is needed in order to prepare for multi bus support.
>
> Signed-off-by: Abel Vesa <abel.vesa@linaro.org>
> ---
> drivers/spmi/spmi-pmic-arb.c | 647 ++++++++++++++++++++++++-------------------
> 1 file changed, 369 insertions(+), 278 deletions(-)
>
> diff --git a/drivers/spmi/spmi-pmic-arb.c b/drivers/spmi/spmi-pmic-arb.c
> index ff777b4a6f33..3db622ed80de 100644
> --- a/drivers/spmi/spmi-pmic-arb.c
> +++ b/drivers/spmi/spmi-pmic-arb.c
> @@ -13,6 +13,7 @@
> #include <linux/kernel.h>
> #include <linux/module.h>
> #include <linux/of.h>
> +#include <linux/of_irq.h>
> #include <linux/platform_device.h>
> #include <linux/slab.h>
> #include <linux/spmi.h>
> @@ -125,61 +126,72 @@ struct apid_data {
> u8 irq_ee;
> };
>
> +struct spmi_pmic_arb;
> +
> /**
> - * struct spmi_pmic_arb - SPMI PMIC Arbiter object
> + * struct spmi_pmic_arb_bus - SPMI PMIC Arbiter Bus object
> *
> - * @rd_base: on v1 "core", on v2 "observer" register base off DT.
> - * @wr_base: on v1 "core", on v2 "chnls" register base off DT.
> + * @pmic_arb: the SPMI PMIC Arbiter the bus belongs to.
> + * @domain: irq domain object for PMIC IRQ domain
> * @intr: address of the SPMI interrupt control registers.
> * @cnfg: address of the PMIC Arbiter configuration registers.
> - * @core: core register base for v2 and above only (see above)
> - * @core_size: core register base size
> - * @lock: lock to synchronize accesses.
> - * @channel: execution environment channel to use for accesses.
> - * @irq: PMIC ARB interrupt.
> - * @ee: the current Execution Environment
> - * @bus_instance: on v7: 0 = primary SPMI bus, 1 = secondary SPMI bus
> - * @min_apid: minimum APID (used for bounding IRQ search)
> - * @max_apid: maximum APID
> + * @spmic: spmi controller registered for this bus
> * @base_apid: on v7: minimum APID associated with the particular SPMI
> * bus instance
> * @apid_count: on v5 and v7: number of APIDs associated with the
> * particular SPMI bus instance
> * @mapping_table: in-memory copy of PPID -> APID mapping table.
> * @mapping_table_valid:bitmap containing valid-only periphs
> - * @domain: irq domain object for PMIC IRQ domain
> - * @spmic: SPMI controller object
> - * @ver_ops: version dependent operations.
> * @ppid_to_apid: in-memory copy of PPID -> APID mapping table.
> * @last_apid: Highest value APID in use
> * @apid_data: Table of data for all APIDs
> + * @min_apid: minimum APID (used for bounding IRQ search)
> + * @max_apid: maximum APID
> + * @irq: PMIC ARB interrupt.
> + */
> +struct spmi_pmic_arb_bus {
> + struct spmi_pmic_arb *pmic_arb;
> + struct irq_domain *domain;
> + void __iomem *intr;
> + void __iomem *cnfg;
> + struct spmi_controller *spmic;
> + u16 base_apid;
> + int apid_count;
> + u32 *mapping_table;
> + DECLARE_BITMAP(mapping_table_valid, PMIC_ARB_MAX_PERIPHS);
> + u16 *ppid_to_apid;
> + u16 last_apid;
> + struct apid_data *apid_data;
> + u16 min_apid;
> + u16 max_apid;
> + int irq;
> +};
> +
> +/**
> + * struct spmi_pmic_arb - SPMI PMIC Arbiter object
> + *
> + * @rd_base: on v1 "core", on v2 "observer" register base off DT.
> + * @wr_base: on v1 "core", on v2 "chnls" register base off DT.
> + * @core: core register base for v2 and above only (see above)
> + * @core_size: core register base size
> + * @lock: lock to synchronize accesses.
> + * @channel: execution environment channel to use for accesses.
> + * @ee: the current Execution Environment
> + * @ver_ops: version dependent operations.
> * @max_periphs: Number of elements in apid_data[]
> + * @bus: per arbiter bus instance
> */
> struct spmi_pmic_arb {
> void __iomem *rd_base;
> void __iomem *wr_base;
> - void __iomem *intr;
> - void __iomem *cnfg;
> void __iomem *core;
> resource_size_t core_size;
> raw_spinlock_t lock;
> u8 channel;
> - int irq;
> u8 ee;
> - u32 bus_instance;
> - u16 min_apid;
> - u16 max_apid;
> - u16 base_apid;
> - int apid_count;
> - u32 *mapping_table;
> - DECLARE_BITMAP(mapping_table_valid, PMIC_ARB_MAX_PERIPHS);
> - struct irq_domain *domain;
> - struct spmi_controller *spmic;
> const struct pmic_arb_ver_ops *ver_ops;
> - u16 *ppid_to_apid;
> - u16 last_apid;
> - struct apid_data *apid_data;
> int max_periphs;
> + struct spmi_pmic_arb_bus *bus;
> };
>
> /**
> @@ -208,21 +220,21 @@ struct spmi_pmic_arb {
> struct pmic_arb_ver_ops {
> const char *ver_str;
> int (*get_core_resources)(struct platform_device *pdev, void __iomem *core);
> - int (*init_apid)(struct spmi_pmic_arb *pmic_arb);
> - int (*ppid_to_apid)(struct spmi_pmic_arb *pmic_arb, u16 ppid);
> + int (*init_apid)(struct spmi_pmic_arb_bus *bus);
> + int (*ppid_to_apid)(struct spmi_pmic_arb_bus *bus, u16 ppid);
> /* spmi commands (read_cmd, write_cmd, cmd) functionality */
> - int (*offset)(struct spmi_pmic_arb *pmic_arb, u8 sid, u16 addr,
> - enum pmic_arb_channel ch_type);
> + int (*offset)(struct spmi_pmic_arb_bus *bus, u8 sid, u16 addr,
> + enum pmic_arb_channel ch_type);
> u32 (*fmt_cmd)(u8 opc, u8 sid, u16 addr, u8 bc);
> int (*non_data_cmd)(struct spmi_controller *ctrl, u8 opc, u8 sid);
> /* Interrupts controller functionality (offset of PIC registers) */
> - void __iomem *(*owner_acc_status)(struct spmi_pmic_arb *pmic_arb, u8 m,
> + void __iomem *(*owner_acc_status)(struct spmi_pmic_arb_bus *bus, u8 m,
> u16 n);
> - void __iomem *(*acc_enable)(struct spmi_pmic_arb *pmic_arb, u16 n);
> - void __iomem *(*irq_status)(struct spmi_pmic_arb *pmic_arb, u16 n);
> - void __iomem *(*irq_clear)(struct spmi_pmic_arb *pmic_arb, u16 n);
> + void __iomem *(*acc_enable)(struct spmi_pmic_arb_bus *bus, u16 n);
> + void __iomem *(*irq_status)(struct spmi_pmic_arb_bus *bus, u16 n);
> + void __iomem *(*irq_clear)(struct spmi_pmic_arb_bus *bus, u16 n);
> u32 (*apid_map_offset)(u16 n);
> - void __iomem *(*apid_owner)(struct spmi_pmic_arb *pmic_arb, u16 n);
> + void __iomem *(*apid_owner)(struct spmi_pmic_arb_bus *bus, u16 n);
> };
>
> static inline void pmic_arb_base_write(struct spmi_pmic_arb *pmic_arb,
> @@ -272,13 +284,14 @@ static int pmic_arb_wait_for_done(struct spmi_controller *ctrl,
> void __iomem *base, u8 sid, u16 addr,
> enum pmic_arb_channel ch_type)
> {
> - struct spmi_pmic_arb *pmic_arb = spmi_controller_get_drvdata(ctrl);
> + struct spmi_pmic_arb_bus *bus = spmi_controller_get_drvdata(ctrl);
> + struct spmi_pmic_arb *pmic_arb = bus->pmic_arb;
> u32 status = 0;
> u32 timeout = PMIC_ARB_TIMEOUT_US;
> u32 offset;
> int rc;
>
> - rc = pmic_arb->ver_ops->offset(pmic_arb, sid, addr, ch_type);
> + rc = pmic_arb->ver_ops->offset(bus, sid, addr, ch_type);
> if (rc < 0)
> return rc;
>
> @@ -321,13 +334,14 @@ static int pmic_arb_wait_for_done(struct spmi_controller *ctrl,
> static int
> pmic_arb_non_data_cmd_v1(struct spmi_controller *ctrl, u8 opc, u8 sid)
> {
> - struct spmi_pmic_arb *pmic_arb = spmi_controller_get_drvdata(ctrl);
> + struct spmi_pmic_arb_bus *bus = spmi_controller_get_drvdata(ctrl);
> + struct spmi_pmic_arb *pmic_arb = bus->pmic_arb;
> unsigned long flags;
> u32 cmd;
> int rc;
> u32 offset;
>
> - rc = pmic_arb->ver_ops->offset(pmic_arb, sid, 0, PMIC_ARB_CHANNEL_RW);
> + rc = pmic_arb->ver_ops->offset(bus, sid, 0, PMIC_ARB_CHANNEL_RW);
> if (rc < 0)
> return rc;
>
> @@ -363,20 +377,21 @@ static int pmic_arb_cmd(struct spmi_controller *ctrl, u8 opc, u8 sid)
> return pmic_arb->ver_ops->non_data_cmd(ctrl, opc, sid);
> }
>
> -static int pmic_arb_fmt_read_cmd(struct spmi_pmic_arb *pmic_arb, u8 opc, u8 sid,
> +static int pmic_arb_fmt_read_cmd(struct spmi_pmic_arb_bus *bus, u8 opc, u8 sid,
> u16 addr, size_t len, u32 *cmd, u32 *offset)
> {
> + struct spmi_pmic_arb *pmic_arb = bus->pmic_arb;
> u8 bc = len - 1;
> int rc;
>
> - rc = pmic_arb->ver_ops->offset(pmic_arb, sid, addr,
> + rc = pmic_arb->ver_ops->offset(bus, sid, addr,
> PMIC_ARB_CHANNEL_OBS);
> if (rc < 0)
> return rc;
>
> *offset = rc;
> if (bc >= PMIC_ARB_MAX_TRANS_BYTES) {
> - dev_err(&pmic_arb->spmic->dev, "pmic-arb supports 1..%d bytes per trans, but:%zu requested",
> + dev_err(&bus->spmic->dev, "pmic-arb supports 1..%d bytes per trans, but:%zu requested",
> PMIC_ARB_MAX_TRANS_BYTES, len);
> return -EINVAL;
> }
> @@ -400,7 +415,8 @@ static int pmic_arb_read_cmd_unlocked(struct spmi_controller *ctrl, u32 cmd,
> u32 offset, u8 sid, u16 addr, u8 *buf,
> size_t len)
> {
> - struct spmi_pmic_arb *pmic_arb = spmi_controller_get_drvdata(ctrl);
> + struct spmi_pmic_arb_bus *bus = spmi_controller_get_drvdata(ctrl);
> + struct spmi_pmic_arb *pmic_arb = bus->pmic_arb;
> u8 bc = len - 1;
> int rc;
>
> @@ -422,12 +438,13 @@ static int pmic_arb_read_cmd_unlocked(struct spmi_controller *ctrl, u32 cmd,
> static int pmic_arb_read_cmd(struct spmi_controller *ctrl, u8 opc, u8 sid,
> u16 addr, u8 *buf, size_t len)
> {
> - struct spmi_pmic_arb *pmic_arb = spmi_controller_get_drvdata(ctrl);
> + struct spmi_pmic_arb_bus *bus = spmi_controller_get_drvdata(ctrl);
> + struct spmi_pmic_arb *pmic_arb = bus->pmic_arb;
> unsigned long flags;
> u32 cmd, offset;
> int rc;
>
> - rc = pmic_arb_fmt_read_cmd(pmic_arb, opc, sid, addr, len, &cmd,
> + rc = pmic_arb_fmt_read_cmd(bus, opc, sid, addr, len, &cmd,
> &offset);
> if (rc)
> return rc;
> @@ -439,21 +456,22 @@ static int pmic_arb_read_cmd(struct spmi_controller *ctrl, u8 opc, u8 sid,
> return rc;
> }
>
> -static int pmic_arb_fmt_write_cmd(struct spmi_pmic_arb *pmic_arb, u8 opc,
> +static int pmic_arb_fmt_write_cmd(struct spmi_pmic_arb_bus *bus, u8 opc,
> u8 sid, u16 addr, size_t len, u32 *cmd,
> u32 *offset)
> {
> + struct spmi_pmic_arb *pmic_arb = bus->pmic_arb;
> u8 bc = len - 1;
> int rc;
>
> - rc = pmic_arb->ver_ops->offset(pmic_arb, sid, addr,
> + rc = pmic_arb->ver_ops->offset(bus, sid, addr,
> PMIC_ARB_CHANNEL_RW);
> if (rc < 0)
> return rc;
>
> *offset = rc;
> if (bc >= PMIC_ARB_MAX_TRANS_BYTES) {
> - dev_err(&pmic_arb->spmic->dev, "pmic-arb supports 1..%d bytes per trans, but:%zu requested",
> + dev_err(&bus->spmic->dev, "pmic-arb supports 1..%d bytes per trans, but:%zu requested",
> PMIC_ARB_MAX_TRANS_BYTES, len);
> return -EINVAL;
> }
> @@ -479,7 +497,8 @@ static int pmic_arb_write_cmd_unlocked(struct spmi_controller *ctrl, u32 cmd,
> u32 offset, u8 sid, u16 addr,
> const u8 *buf, size_t len)
> {
> - struct spmi_pmic_arb *pmic_arb = spmi_controller_get_drvdata(ctrl);
> + struct spmi_pmic_arb_bus *bus = spmi_controller_get_drvdata(ctrl);
> + struct spmi_pmic_arb *pmic_arb = bus->pmic_arb;
> u8 bc = len - 1;
>
> /* Write data to FIFOs */
> @@ -498,12 +517,13 @@ static int pmic_arb_write_cmd_unlocked(struct spmi_controller *ctrl, u32 cmd,
> static int pmic_arb_write_cmd(struct spmi_controller *ctrl, u8 opc, u8 sid,
> u16 addr, const u8 *buf, size_t len)
> {
> - struct spmi_pmic_arb *pmic_arb = spmi_controller_get_drvdata(ctrl);
> + struct spmi_pmic_arb_bus *bus = spmi_controller_get_drvdata(ctrl);
> + struct spmi_pmic_arb *pmic_arb = bus->pmic_arb;
> unsigned long flags;
> u32 cmd, offset;
> int rc;
>
> - rc = pmic_arb_fmt_write_cmd(pmic_arb, opc, sid, addr, len, &cmd,
> + rc = pmic_arb_fmt_write_cmd(bus, opc, sid, addr, len, &cmd,
> &offset);
> if (rc)
> return rc;
> @@ -519,18 +539,19 @@ static int pmic_arb_write_cmd(struct spmi_controller *ctrl, u8 opc, u8 sid,
> static int pmic_arb_masked_write(struct spmi_controller *ctrl, u8 sid, u16 addr,
> const u8 *buf, const u8 *mask, size_t len)
> {
> - struct spmi_pmic_arb *pmic_arb = spmi_controller_get_drvdata(ctrl);
> + struct spmi_pmic_arb_bus *bus = spmi_controller_get_drvdata(ctrl);
> + struct spmi_pmic_arb *pmic_arb = bus->pmic_arb;
> u32 read_cmd, read_offset, write_cmd, write_offset;
> u8 temp[PMIC_ARB_MAX_TRANS_BYTES];
> unsigned long flags;
> int rc, i;
>
> - rc = pmic_arb_fmt_read_cmd(pmic_arb, SPMI_CMD_EXT_READL, sid, addr, len,
> + rc = pmic_arb_fmt_read_cmd(bus, SPMI_CMD_EXT_READL, sid, addr, len,
> &read_cmd, &read_offset);
> if (rc)
> return rc;
>
> - rc = pmic_arb_fmt_write_cmd(pmic_arb, SPMI_CMD_EXT_WRITEL, sid, addr,
> + rc = pmic_arb_fmt_write_cmd(bus, SPMI_CMD_EXT_WRITEL, sid, addr,
> len, &write_cmd, &write_offset);
> if (rc)
> return rc;
> @@ -573,25 +594,25 @@ struct spmi_pmic_arb_qpnpint_type {
> static void qpnpint_spmi_write(struct irq_data *d, u8 reg, void *buf,
> size_t len)
> {
> - struct spmi_pmic_arb *pmic_arb = irq_data_get_irq_chip_data(d);
> + struct spmi_pmic_arb_bus *bus = irq_data_get_irq_chip_data(d);
> u8 sid = hwirq_to_sid(d->hwirq);
> u8 per = hwirq_to_per(d->hwirq);
>
> - if (pmic_arb_write_cmd(pmic_arb->spmic, SPMI_CMD_EXT_WRITEL, sid,
> + if (pmic_arb_write_cmd(bus->spmic, SPMI_CMD_EXT_WRITEL, sid,
> (per << 8) + reg, buf, len))
> - dev_err_ratelimited(&pmic_arb->spmic->dev, "failed irqchip transaction on %x\n",
> + dev_err_ratelimited(&bus->spmic->dev, "failed irqchip transaction on %x\n",
> d->irq);
> }
>
> static void qpnpint_spmi_read(struct irq_data *d, u8 reg, void *buf, size_t len)
> {
> - struct spmi_pmic_arb *pmic_arb = irq_data_get_irq_chip_data(d);
> + struct spmi_pmic_arb_bus *bus = irq_data_get_irq_chip_data(d);
> u8 sid = hwirq_to_sid(d->hwirq);
> u8 per = hwirq_to_per(d->hwirq);
>
> - if (pmic_arb_read_cmd(pmic_arb->spmic, SPMI_CMD_EXT_READL, sid,
> + if (pmic_arb_read_cmd(bus->spmic, SPMI_CMD_EXT_READL, sid,
> (per << 8) + reg, buf, len))
> - dev_err_ratelimited(&pmic_arb->spmic->dev, "failed irqchip transaction on %x\n",
> + dev_err_ratelimited(&bus->spmic->dev, "failed irqchip transaction on %x\n",
> d->irq);
> }
>
> @@ -599,47 +620,49 @@ static int qpnpint_spmi_masked_write(struct irq_data *d, u8 reg,
> const void *buf, const void *mask,
> size_t len)
> {
> - struct spmi_pmic_arb *pmic_arb = irq_data_get_irq_chip_data(d);
> + struct spmi_pmic_arb_bus *bus = irq_data_get_irq_chip_data(d);
> u8 sid = hwirq_to_sid(d->hwirq);
> u8 per = hwirq_to_per(d->hwirq);
> int rc;
>
> - rc = pmic_arb_masked_write(pmic_arb->spmic, sid, (per << 8) + reg, buf,
> + rc = pmic_arb_masked_write(bus->spmic, sid, (per << 8) + reg, buf,
> mask, len);
> if (rc)
> - dev_err_ratelimited(&pmic_arb->spmic->dev, "failed irqchip transaction on %x rc=%d\n",
> + dev_err_ratelimited(&bus->spmic->dev, "failed irqchip transaction on %x rc=%d\n",
> d->irq, rc);
> return rc;
> }
>
> -static void cleanup_irq(struct spmi_pmic_arb *pmic_arb, u16 apid, int id)
> +static void cleanup_irq(struct spmi_pmic_arb_bus *bus, u16 apid, int id)
> {
> - u16 ppid = pmic_arb->apid_data[apid].ppid;
> + struct spmi_pmic_arb *pmic_arb = bus->pmic_arb;
> + u16 ppid = bus->apid_data[apid].ppid;
> u8 sid = ppid >> 8;
> u8 per = ppid & 0xFF;
> u8 irq_mask = BIT(id);
>
> - dev_err_ratelimited(&pmic_arb->spmic->dev, "%s apid=%d sid=0x%x per=0x%x irq=%d\n",
> - __func__, apid, sid, per, id);
> - writel_relaxed(irq_mask, pmic_arb->ver_ops->irq_clear(pmic_arb, apid));
> + dev_err_ratelimited(&bus->spmic->dev, "%s apid=%d sid=0x%x per=0x%x irq=%d\n",
> + __func__, apid, sid, per, id);
> + writel_relaxed(irq_mask, pmic_arb->ver_ops->irq_clear(bus, apid));
> }
>
> -static int periph_interrupt(struct spmi_pmic_arb *pmic_arb, u16 apid)
> +static int periph_interrupt(struct spmi_pmic_arb_bus *bus, u16 apid)
> {
> + struct spmi_pmic_arb *pmic_arb = bus->pmic_arb;
> unsigned int irq;
> u32 status, id;
> int handled = 0;
> - u8 sid = (pmic_arb->apid_data[apid].ppid >> 8) & 0xF;
> - u8 per = pmic_arb->apid_data[apid].ppid & 0xFF;
> + u8 sid = (bus->apid_data[apid].ppid >> 8) & 0xF;
> + u8 per = bus->apid_data[apid].ppid & 0xFF;
>
> - status = readl_relaxed(pmic_arb->ver_ops->irq_status(pmic_arb, apid));
> + status = readl_relaxed(pmic_arb->ver_ops->irq_status(bus, apid));
> while (status) {
> id = ffs(status) - 1;
> status &= ~BIT(id);
> - irq = irq_find_mapping(pmic_arb->domain,
> - spec_to_hwirq(sid, per, id, apid));
> + irq = irq_find_mapping(bus->domain,
> + spec_to_hwirq(sid, per, id, apid));
> if (irq == 0) {
> - cleanup_irq(pmic_arb, apid, id);
> + cleanup_irq(bus, apid, id);
> continue;
> }
> generic_handle_irq(irq);
> @@ -651,16 +674,17 @@ static int periph_interrupt(struct spmi_pmic_arb *pmic_arb, u16 apid)
>
> static void pmic_arb_chained_irq(struct irq_desc *desc)
> {
> - struct spmi_pmic_arb *pmic_arb = irq_desc_get_handler_data(desc);
> + struct spmi_pmic_arb_bus *bus = irq_desc_get_handler_data(desc);
> + struct spmi_pmic_arb *pmic_arb = bus->pmic_arb;
> const struct pmic_arb_ver_ops *ver_ops = pmic_arb->ver_ops;
> struct irq_chip *chip = irq_desc_get_chip(desc);
> - int first = pmic_arb->min_apid;
> - int last = pmic_arb->max_apid;
> + int first = bus->min_apid;
> + int last = bus->max_apid;
> /*
> * acc_offset will be non-zero for the secondary SPMI bus instance on
> * v7 controllers.
> */
> - int acc_offset = pmic_arb->base_apid >> 5;
> + int acc_offset = bus->base_apid >> 5;
> u8 ee = pmic_arb->ee;
> u32 status, enable, handled = 0;
> int i, id, apid;
> @@ -671,7 +695,7 @@ static void pmic_arb_chained_irq(struct irq_desc *desc)
> chained_irq_enter(chip, desc);
>
> for (i = first >> 5; i <= last >> 5; ++i) {
> - status = readl_relaxed(ver_ops->owner_acc_status(pmic_arb, ee, i - acc_offset));
> + status = readl_relaxed(ver_ops->owner_acc_status(bus, ee, i - acc_offset));
> if (status)
> acc_valid = true;
>
> @@ -685,9 +709,9 @@ static void pmic_arb_chained_irq(struct irq_desc *desc)
> continue;
> }
> enable = readl_relaxed(
> - ver_ops->acc_enable(pmic_arb, apid));
> + ver_ops->acc_enable(bus, apid));
> if (enable & SPMI_PIC_ACC_ENABLE_BIT)
> - if (periph_interrupt(pmic_arb, apid) != 0)
> + if (periph_interrupt(bus, apid) != 0)
> handled++;
> }
> }
> @@ -696,19 +720,19 @@ static void pmic_arb_chained_irq(struct irq_desc *desc)
> if (!acc_valid) {
> for (i = first; i <= last; i++) {
> /* skip if APPS is not irq owner */
> - if (pmic_arb->apid_data[i].irq_ee != pmic_arb->ee)
> + if (bus->apid_data[i].irq_ee != pmic_arb->ee)
> continue;
>
> irq_status = readl_relaxed(
> - ver_ops->irq_status(pmic_arb, i));
> + ver_ops->irq_status(bus, i));
> if (irq_status) {
> enable = readl_relaxed(
> - ver_ops->acc_enable(pmic_arb, i));
> + ver_ops->acc_enable(bus, i));
> if (enable & SPMI_PIC_ACC_ENABLE_BIT) {
> - dev_dbg(&pmic_arb->spmic->dev,
> + dev_dbg(&bus->spmic->dev,
> "Dispatching IRQ for apid=%d status=%x\n",
> i, irq_status);
> - if (periph_interrupt(pmic_arb, i) != 0)
> + if (periph_interrupt(bus, i) != 0)
> handled++;
> }
> }
> @@ -723,12 +747,13 @@ static void pmic_arb_chained_irq(struct irq_desc *desc)
>
> static void qpnpint_irq_ack(struct irq_data *d)
> {
> - struct spmi_pmic_arb *pmic_arb = irq_data_get_irq_chip_data(d);
> + struct spmi_pmic_arb_bus *bus = irq_data_get_irq_chip_data(d);
> + struct spmi_pmic_arb *pmic_arb = bus->pmic_arb;
> u8 irq = hwirq_to_irq(d->hwirq);
> u16 apid = hwirq_to_apid(d->hwirq);
> u8 data;
>
> - writel_relaxed(BIT(irq), pmic_arb->ver_ops->irq_clear(pmic_arb, apid));
> + writel_relaxed(BIT(irq), pmic_arb->ver_ops->irq_clear(bus, apid));
>
> data = BIT(irq);
> qpnpint_spmi_write(d, QPNPINT_REG_LATCHED_CLR, &data, 1);
> @@ -744,14 +769,15 @@ static void qpnpint_irq_mask(struct irq_data *d)
>
> static void qpnpint_irq_unmask(struct irq_data *d)
> {
> - struct spmi_pmic_arb *pmic_arb = irq_data_get_irq_chip_data(d);
> + struct spmi_pmic_arb_bus *bus = irq_data_get_irq_chip_data(d);
> + struct spmi_pmic_arb *pmic_arb = bus->pmic_arb;
> const struct pmic_arb_ver_ops *ver_ops = pmic_arb->ver_ops;
> u8 irq = hwirq_to_irq(d->hwirq);
> u16 apid = hwirq_to_apid(d->hwirq);
> u8 buf[2];
>
> writel_relaxed(SPMI_PIC_ACC_ENABLE_BIT,
> - ver_ops->acc_enable(pmic_arb, apid));
> + ver_ops->acc_enable(bus, apid));
>
> qpnpint_spmi_read(d, QPNPINT_REG_EN_SET, &buf[0], 1);
> if (!(buf[0] & BIT(irq))) {
> @@ -808,9 +834,9 @@ static int qpnpint_irq_set_type(struct irq_data *d, unsigned int flow_type)
>
> static int qpnpint_irq_set_wake(struct irq_data *d, unsigned int on)
> {
> - struct spmi_pmic_arb *pmic_arb = irq_data_get_irq_chip_data(d);
> + struct spmi_pmic_arb_bus *bus = irq_data_get_irq_chip_data(d);
>
> - return irq_set_irq_wake(pmic_arb->irq, on);
> + return irq_set_irq_wake(bus->irq, on);
> }
>
> static int qpnpint_get_irqchip_state(struct irq_data *d,
> @@ -832,17 +858,18 @@ static int qpnpint_get_irqchip_state(struct irq_data *d,
> static int qpnpint_irq_domain_activate(struct irq_domain *domain,
> struct irq_data *d, bool reserve)
> {
> - struct spmi_pmic_arb *pmic_arb = irq_data_get_irq_chip_data(d);
> + struct spmi_pmic_arb_bus *bus = irq_data_get_irq_chip_data(d);
> + struct spmi_pmic_arb *pmic_arb = bus->pmic_arb;
> u16 periph = hwirq_to_per(d->hwirq);
> u16 apid = hwirq_to_apid(d->hwirq);
> u16 sid = hwirq_to_sid(d->hwirq);
> u16 irq = hwirq_to_irq(d->hwirq);
> u8 buf;
>
> - if (pmic_arb->apid_data[apid].irq_ee != pmic_arb->ee) {
> - dev_err(&pmic_arb->spmic->dev, "failed to xlate sid = %#x, periph = %#x, irq = %u: ee=%u but owner=%u\n",
> + if (bus->apid_data[apid].irq_ee != pmic_arb->ee) {
> + dev_err(&bus->spmic->dev, "failed to xlate sid = %#x, periph = %#x, irq = %u: ee=%u but owner=%u\n",
> sid, periph, irq, pmic_arb->ee,
> - pmic_arb->apid_data[apid].irq_ee);
> + bus->apid_data[apid].irq_ee);
> return -ENODEV;
> }
>
> @@ -869,15 +896,16 @@ static int qpnpint_irq_domain_translate(struct irq_domain *d,
> unsigned long *out_hwirq,
> unsigned int *out_type)
> {
> - struct spmi_pmic_arb *pmic_arb = d->host_data;
> + struct spmi_pmic_arb_bus *bus = d->host_data;
> + struct spmi_pmic_arb *pmic_arb = bus->pmic_arb;
> u32 *intspec = fwspec->param;
> u16 apid, ppid;
> int rc;
>
> - dev_dbg(&pmic_arb->spmic->dev, "intspec[0] 0x%1x intspec[1] 0x%02x intspec[2] 0x%02x\n",
> + dev_dbg(&bus->spmic->dev, "intspec[0] 0x%1x intspec[1] 0x%02x intspec[2] 0x%02x\n",
> intspec[0], intspec[1], intspec[2]);
>
> - if (irq_domain_get_of_node(d) != pmic_arb->spmic->dev.of_node)
> + if (irq_domain_get_of_node(d) != bus->spmic->dev.of_node)
> return -EINVAL;
> if (fwspec->param_count != 4)
> return -EINVAL;
> @@ -885,37 +913,37 @@ static int qpnpint_irq_domain_translate(struct irq_domain *d,
> return -EINVAL;
>
> ppid = intspec[0] << 8 | intspec[1];
> - rc = pmic_arb->ver_ops->ppid_to_apid(pmic_arb, ppid);
> + rc = pmic_arb->ver_ops->ppid_to_apid(bus, ppid);
> if (rc < 0) {
> - dev_err(&pmic_arb->spmic->dev, "failed to xlate sid = %#x, periph = %#x, irq = %u rc = %d\n",
> - intspec[0], intspec[1], intspec[2], rc);
> + dev_err(&bus->spmic->dev, "failed to xlate sid = %#x, periph = %#x, irq = %u rc = %d\n",
> + intspec[0], intspec[1], intspec[2], rc);
> return rc;
> }
>
> apid = rc;
> /* Keep track of {max,min}_apid for bounding search during interrupt */
> - if (apid > pmic_arb->max_apid)
> - pmic_arb->max_apid = apid;
> - if (apid < pmic_arb->min_apid)
> - pmic_arb->min_apid = apid;
> + if (apid > bus->max_apid)
> + bus->max_apid = apid;
> + if (apid < bus->min_apid)
> + bus->min_apid = apid;
>
> *out_hwirq = spec_to_hwirq(intspec[0], intspec[1], intspec[2], apid);
> *out_type = intspec[3] & IRQ_TYPE_SENSE_MASK;
>
> - dev_dbg(&pmic_arb->spmic->dev, "out_hwirq = %lu\n", *out_hwirq);
> + dev_dbg(&bus->spmic->dev, "out_hwirq = %lu\n", *out_hwirq);
>
> return 0;
> }
>
> static struct lock_class_key qpnpint_irq_lock_class, qpnpint_irq_request_class;
>
> -static void qpnpint_irq_domain_map(struct spmi_pmic_arb *pmic_arb,
> +static void qpnpint_irq_domain_map(struct spmi_pmic_arb_bus *bus,
> struct irq_domain *domain, unsigned int virq,
> irq_hw_number_t hwirq, unsigned int type)
> {
> irq_flow_handler_t handler;
>
> - dev_dbg(&pmic_arb->spmic->dev, "virq = %u, hwirq = %lu, type = %u\n",
> + dev_dbg(&bus->spmic->dev, "virq = %u, hwirq = %lu, type = %u\n",
> virq, hwirq, type);
>
> if (type & IRQ_TYPE_EDGE_BOTH)
> @@ -926,7 +954,7 @@ static void qpnpint_irq_domain_map(struct spmi_pmic_arb *pmic_arb,
>
> irq_set_lockdep_class(virq, &qpnpint_irq_lock_class,
> &qpnpint_irq_request_class);
> - irq_domain_set_info(domain, virq, hwirq, &pmic_arb_irqchip, pmic_arb,
> + irq_domain_set_info(domain, virq, hwirq, &pmic_arb_irqchip, bus,
> handler, NULL, NULL);
> }
>
> @@ -934,7 +962,7 @@ static int qpnpint_irq_domain_alloc(struct irq_domain *domain,
> unsigned int virq, unsigned int nr_irqs,
> void *data)
> {
> - struct spmi_pmic_arb *pmic_arb = domain->host_data;
> + struct spmi_pmic_arb_bus *bus = domain->host_data;
> struct irq_fwspec *fwspec = data;
> irq_hw_number_t hwirq;
> unsigned int type;
> @@ -945,20 +973,22 @@ static int qpnpint_irq_domain_alloc(struct irq_domain *domain,
> return ret;
>
> for (i = 0; i < nr_irqs; i++)
> - qpnpint_irq_domain_map(pmic_arb, domain, virq + i, hwirq + i,
> + qpnpint_irq_domain_map(bus, domain, virq + i, hwirq + i,
> type);
>
> return 0;
> }
>
> -static int pmic_arb_init_apid_min_max(struct spmi_pmic_arb *pmic_arb)
> +static int pmic_arb_init_apid_min_max(struct spmi_pmic_arb_bus *bus)
> {
> + struct spmi_pmic_arb *pmic_arb = bus->pmic_arb;
> +
> /*
> * Initialize max_apid/min_apid to the opposite bounds, during
> * the irq domain translation, we are sure to update these
> */
> - pmic_arb->max_apid = 0;
> - pmic_arb->min_apid = pmic_arb->max_periphs - 1;
> + bus->max_apid = 0;
> + bus->min_apid = pmic_arb->max_periphs - 1;
>
> return 0;
> }
> @@ -976,37 +1006,38 @@ static int pmic_arb_get_core_resources_v1(struct platform_device *pdev,
> return 0;
> }
>
> -static int pmic_arb_init_apid_v1(struct spmi_pmic_arb *pmic_arb)
> +static int pmic_arb_init_apid_v1(struct spmi_pmic_arb_bus *bus)
> {
> + struct spmi_pmic_arb *pmic_arb = bus->pmic_arb;
> u32 *mapping_table;
>
> - mapping_table = devm_kcalloc(&pmic_arb->spmic->dev, pmic_arb->max_periphs,
> + mapping_table = devm_kcalloc(&bus->spmic->dev, pmic_arb->max_periphs,
> sizeof(*mapping_table), GFP_KERNEL);
> if (!mapping_table)
> return -ENOMEM;
>
> - pmic_arb->mapping_table = mapping_table;
> + bus->mapping_table = mapping_table;
>
> - return pmic_arb_init_apid_min_max(pmic_arb);
> + return pmic_arb_init_apid_min_max(bus);
> }
>
> -static int pmic_arb_ppid_to_apid_v1(struct spmi_pmic_arb *pmic_arb, u16 ppid)
> +static int pmic_arb_ppid_to_apid_v1(struct spmi_pmic_arb_bus *bus, u16 ppid)
> {
> - u32 *mapping_table = pmic_arb->mapping_table;
> + u32 *mapping_table = bus->mapping_table;
> int index = 0, i;
> u16 apid_valid;
> u16 apid;
> u32 data;
>
> - apid_valid = pmic_arb->ppid_to_apid[ppid];
> + apid_valid = bus->ppid_to_apid[ppid];
> if (apid_valid & PMIC_ARB_APID_VALID) {
> apid = apid_valid & ~PMIC_ARB_APID_VALID;
> return apid;
> }
>
> for (i = 0; i < SPMI_MAPPING_TABLE_TREE_DEPTH; ++i) {
> - if (!test_and_set_bit(index, pmic_arb->mapping_table_valid))
> - mapping_table[index] = readl_relaxed(pmic_arb->cnfg +
> + if (!test_and_set_bit(index, bus->mapping_table_valid))
> + mapping_table[index] = readl_relaxed(bus->cnfg +
> SPMI_MAPPING_TABLE_REG(index));
>
> data = mapping_table[index];
> @@ -1016,9 +1047,9 @@ static int pmic_arb_ppid_to_apid_v1(struct spmi_pmic_arb *pmic_arb, u16 ppid)
> index = SPMI_MAPPING_BIT_IS_1_RESULT(data);
> } else {
> apid = SPMI_MAPPING_BIT_IS_1_RESULT(data);
> - pmic_arb->ppid_to_apid[ppid]
> + bus->ppid_to_apid[ppid]
> = apid | PMIC_ARB_APID_VALID;
> - pmic_arb->apid_data[apid].ppid = ppid;
> + bus->apid_data[apid].ppid = ppid;
> return apid;
> }
> } else {
> @@ -1026,9 +1057,9 @@ static int pmic_arb_ppid_to_apid_v1(struct spmi_pmic_arb *pmic_arb, u16 ppid)
> index = SPMI_MAPPING_BIT_IS_0_RESULT(data);
> } else {
> apid = SPMI_MAPPING_BIT_IS_0_RESULT(data);
> - pmic_arb->ppid_to_apid[ppid]
> + bus->ppid_to_apid[ppid]
> = apid | PMIC_ARB_APID_VALID;
> - pmic_arb->apid_data[apid].ppid = ppid;
> + bus->apid_data[apid].ppid = ppid;
> return apid;
> }
> }
> @@ -1038,24 +1069,26 @@ static int pmic_arb_ppid_to_apid_v1(struct spmi_pmic_arb *pmic_arb, u16 ppid)
> }
>
> /* v1 offset per ee */
> -static int pmic_arb_offset_v1(struct spmi_pmic_arb *pmic_arb, u8 sid, u16 addr,
> - enum pmic_arb_channel ch_type)
> +static int pmic_arb_offset_v1(struct spmi_pmic_arb_bus *bus, u8 sid, u16 addr,
> + enum pmic_arb_channel ch_type)
> {
> + struct spmi_pmic_arb *pmic_arb = bus->pmic_arb;
> return 0x800 + 0x80 * pmic_arb->channel;
> }
>
> -static u16 pmic_arb_find_apid(struct spmi_pmic_arb *pmic_arb, u16 ppid)
> +static u16 pmic_arb_find_apid(struct spmi_pmic_arb_bus *bus, u16 ppid)
> {
> - struct apid_data *apidd = &pmic_arb->apid_data[pmic_arb->last_apid];
> + struct spmi_pmic_arb *pmic_arb = bus->pmic_arb;
> + struct apid_data *apidd = &bus->apid_data[bus->last_apid];
> u32 regval, offset;
> u16 id, apid;
>
> - for (apid = pmic_arb->last_apid; ; apid++, apidd++) {
> + for (apid = bus->last_apid; ; apid++, apidd++) {
> offset = pmic_arb->ver_ops->apid_map_offset(apid);
> if (offset >= pmic_arb->core_size)
> break;
>
> - regval = readl_relaxed(pmic_arb->ver_ops->apid_owner(pmic_arb,
> + regval = readl_relaxed(pmic_arb->ver_ops->apid_owner(bus,
> apid));
> apidd->irq_ee = SPMI_OWNERSHIP_PERIPH2OWNER(regval);
> apidd->write_ee = apidd->irq_ee;
> @@ -1065,14 +1098,14 @@ static u16 pmic_arb_find_apid(struct spmi_pmic_arb *pmic_arb, u16 ppid)
> continue;
>
> id = (regval >> 8) & PMIC_ARB_PPID_MASK;
> - pmic_arb->ppid_to_apid[id] = apid | PMIC_ARB_APID_VALID;
> + bus->ppid_to_apid[id] = apid | PMIC_ARB_APID_VALID;
> apidd->ppid = id;
> if (id == ppid) {
> apid |= PMIC_ARB_APID_VALID;
> break;
> }
> }
> - pmic_arb->last_apid = apid & ~PMIC_ARB_APID_VALID;
> + bus->last_apid = apid & ~PMIC_ARB_APID_VALID;
>
> return apid;
> }
> @@ -1104,21 +1137,22 @@ static int pmic_arb_get_core_resources_v2(struct platform_device *pdev,
> return pmic_arb_get_obsrvr_chnls_v2(pdev);
> }
>
> -static int pmic_arb_ppid_to_apid_v2(struct spmi_pmic_arb *pmic_arb, u16 ppid)
> +static int pmic_arb_ppid_to_apid_v2(struct spmi_pmic_arb_bus *bus, u16 ppid)
> {
> u16 apid_valid;
>
> - apid_valid = pmic_arb->ppid_to_apid[ppid];
> + apid_valid = bus->ppid_to_apid[ppid];
> if (!(apid_valid & PMIC_ARB_APID_VALID))
> - apid_valid = pmic_arb_find_apid(pmic_arb, ppid);
> + apid_valid = pmic_arb_find_apid(bus, ppid);
> if (!(apid_valid & PMIC_ARB_APID_VALID))
> return -ENODEV;
>
> return apid_valid & ~PMIC_ARB_APID_VALID;
> }
>
> -static int pmic_arb_read_apid_map_v5(struct spmi_pmic_arb *pmic_arb)
> +static int pmic_arb_read_apid_map_v5(struct spmi_pmic_arb_bus *bus)
> {
> + struct spmi_pmic_arb *pmic_arb = bus->pmic_arb;
> struct apid_data *apidd;
> struct apid_data *prev_apidd;
> u16 i, apid, ppid, apid_max;
> @@ -1140,9 +1174,9 @@ static int pmic_arb_read_apid_map_v5(struct spmi_pmic_arb *pmic_arb)
> * where N = number of APIDs supported by the primary bus and
> * M = number of APIDs supported by the secondary bus
> */
> - apidd = &pmic_arb->apid_data[pmic_arb->base_apid];
> - apid_max = pmic_arb->base_apid + pmic_arb->apid_count;
> - for (i = pmic_arb->base_apid; i < apid_max; i++, apidd++) {
> + apidd = &bus->apid_data[bus->base_apid];
> + apid_max = bus->base_apid + bus->apid_count;
> + for (i = bus->base_apid; i < apid_max; i++, apidd++) {
> offset = pmic_arb->ver_ops->apid_map_offset(i);
> if (offset >= pmic_arb->core_size)
> break;
> @@ -1153,19 +1187,18 @@ static int pmic_arb_read_apid_map_v5(struct spmi_pmic_arb *pmic_arb)
> ppid = (regval >> 8) & PMIC_ARB_PPID_MASK;
> is_irq_ee = PMIC_ARB_CHAN_IS_IRQ_OWNER(regval);
>
> - regval = readl_relaxed(pmic_arb->ver_ops->apid_owner(pmic_arb,
> - i));
> + regval = readl_relaxed(pmic_arb->ver_ops->apid_owner(bus, i));
> apidd->write_ee = SPMI_OWNERSHIP_PERIPH2OWNER(regval);
>
> apidd->irq_ee = is_irq_ee ? apidd->write_ee : INVALID_EE;
>
> - valid = pmic_arb->ppid_to_apid[ppid] & PMIC_ARB_APID_VALID;
> - apid = pmic_arb->ppid_to_apid[ppid] & ~PMIC_ARB_APID_VALID;
> - prev_apidd = &pmic_arb->apid_data[apid];
> + valid = bus->ppid_to_apid[ppid] & PMIC_ARB_APID_VALID;
> + apid = bus->ppid_to_apid[ppid] & ~PMIC_ARB_APID_VALID;
> + prev_apidd = &bus->apid_data[apid];
>
> if (!valid || apidd->write_ee == pmic_arb->ee) {
> /* First PPID mapping or one for this EE */
> - pmic_arb->ppid_to_apid[ppid] = i | PMIC_ARB_APID_VALID;
> + bus->ppid_to_apid[ppid] = i | PMIC_ARB_APID_VALID;
> } else if (valid && is_irq_ee &&
> prev_apidd->write_ee == pmic_arb->ee) {
> /*
> @@ -1176,42 +1209,43 @@ static int pmic_arb_read_apid_map_v5(struct spmi_pmic_arb *pmic_arb)
> }
>
> apidd->ppid = ppid;
> - pmic_arb->last_apid = i;
> + bus->last_apid = i;
> }
>
> /* Dump the mapping table for debug purposes. */
> - dev_dbg(&pmic_arb->spmic->dev, "PPID APID Write-EE IRQ-EE\n");
> + dev_dbg(&bus->spmic->dev, "PPID APID Write-EE IRQ-EE\n");
> for (ppid = 0; ppid < PMIC_ARB_MAX_PPID; ppid++) {
> - apid = pmic_arb->ppid_to_apid[ppid];
> + apid = bus->ppid_to_apid[ppid];
> if (apid & PMIC_ARB_APID_VALID) {
> apid &= ~PMIC_ARB_APID_VALID;
> - apidd = &pmic_arb->apid_data[apid];
> - dev_dbg(&pmic_arb->spmic->dev, "%#03X %3u %2u %2u\n",
> - ppid, apid, apidd->write_ee, apidd->irq_ee);
> + apidd = &bus->apid_data[apid];
> + dev_dbg(&bus->spmic->dev, "%#03X %3u %2u %2u\n",
> + ppid, apid, apidd->write_ee, apidd->irq_ee);
> }
> }
>
> return 0;
> }
>
> -static int pmic_arb_ppid_to_apid_v5(struct spmi_pmic_arb *pmic_arb, u16 ppid)
> +static int pmic_arb_ppid_to_apid_v5(struct spmi_pmic_arb_bus *bus, u16 ppid)
> {
> - if (!(pmic_arb->ppid_to_apid[ppid] & PMIC_ARB_APID_VALID))
> + if (!(bus->ppid_to_apid[ppid] & PMIC_ARB_APID_VALID))
> return -ENODEV;
>
> - return pmic_arb->ppid_to_apid[ppid] & ~PMIC_ARB_APID_VALID;
> + return bus->ppid_to_apid[ppid] & ~PMIC_ARB_APID_VALID;
> }
>
> /* v2 offset per ppid and per ee */
> -static int pmic_arb_offset_v2(struct spmi_pmic_arb *pmic_arb, u8 sid, u16 addr,
> - enum pmic_arb_channel ch_type)
> +static int pmic_arb_offset_v2(struct spmi_pmic_arb_bus *bus, u8 sid, u16 addr,
> + enum pmic_arb_channel ch_type)
> {
> + struct spmi_pmic_arb *pmic_arb = bus->pmic_arb;
> u16 apid;
> u16 ppid;
> int rc;
>
> ppid = sid << 8 | ((addr >> 8) & 0xFF);
> - rc = pmic_arb_ppid_to_apid_v2(pmic_arb, ppid);
> + rc = pmic_arb_ppid_to_apid_v2(bus, ppid);
> if (rc < 0)
> return rc;
>
> @@ -1219,27 +1253,28 @@ static int pmic_arb_offset_v2(struct spmi_pmic_arb *pmic_arb, u8 sid, u16 addr,
> return 0x1000 * pmic_arb->ee + 0x8000 * apid;
> }
>
> -static int pmic_arb_init_apid_v5(struct spmi_pmic_arb *pmic_arb)
> +static int pmic_arb_init_apid_v5(struct spmi_pmic_arb_bus *bus)
> {
> + struct spmi_pmic_arb *pmic_arb = bus->pmic_arb;
> int ret;
>
> - pmic_arb->base_apid = 0;
> - pmic_arb->apid_count = readl_relaxed(pmic_arb->core + PMIC_ARB_FEATURES) &
> - PMIC_ARB_FEATURES_PERIPH_MASK;
> + bus->base_apid = 0;
> + bus->apid_count = readl_relaxed(pmic_arb->core + PMIC_ARB_FEATURES) &
> + PMIC_ARB_FEATURES_PERIPH_MASK;
>
> - if (pmic_arb->base_apid + pmic_arb->apid_count > pmic_arb->max_periphs) {
> - dev_err(&pmic_arb->spmic->dev, "Unsupported APID count %d detected\n",
> - pmic_arb->base_apid + pmic_arb->apid_count);
> + if (bus->base_apid + bus->apid_count > pmic_arb->max_periphs) {
> + dev_err(&bus->spmic->dev, "Unsupported APID count %d detected\n",
> + bus->base_apid + bus->apid_count);
> return -EINVAL;
> }
>
> - ret = pmic_arb_init_apid_min_max(pmic_arb);
> + ret = pmic_arb_init_apid_min_max(bus);
> if (ret)
> return ret;
>
> - ret = pmic_arb_read_apid_map_v5(pmic_arb);
> + ret = pmic_arb_read_apid_map_v5(bus);
> if (ret) {
> - dev_err(&pmic_arb->spmic->dev, "could not read APID->PPID mapping table, rc= %d\n",
> + dev_err(&bus->spmic->dev, "could not read APID->PPID mapping table, rc= %d\n",
> ret);
> return ret;
> }
> @@ -1251,15 +1286,16 @@ static int pmic_arb_init_apid_v5(struct spmi_pmic_arb *pmic_arb)
> * v5 offset per ee and per apid for observer channels and per apid for
> * read/write channels.
> */
> -static int pmic_arb_offset_v5(struct spmi_pmic_arb *pmic_arb, u8 sid, u16 addr,
> - enum pmic_arb_channel ch_type)
> +static int pmic_arb_offset_v5(struct spmi_pmic_arb_bus *bus, u8 sid, u16 addr,
> + enum pmic_arb_channel ch_type)
> {
> + struct spmi_pmic_arb *pmic_arb = bus->pmic_arb;
> u16 apid;
> int rc;
> u32 offset = 0;
> u16 ppid = (sid << 8) | (addr >> 8);
>
> - rc = pmic_arb_ppid_to_apid_v5(pmic_arb, ppid);
> + rc = pmic_arb_ppid_to_apid_v5(bus, ppid);
> if (rc < 0)
> return rc;
>
> @@ -1269,8 +1305,8 @@ static int pmic_arb_offset_v5(struct spmi_pmic_arb *pmic_arb, u8 sid, u16 addr,
> offset = 0x10000 * pmic_arb->ee + 0x80 * apid;
> break;
> case PMIC_ARB_CHANNEL_RW:
> - if (pmic_arb->apid_data[apid].write_ee != pmic_arb->ee) {
> - dev_err(&pmic_arb->spmic->dev, "disallowed SPMI write to sid=%u, addr=0x%04X\n",
> + if (bus->apid_data[apid].write_ee != pmic_arb->ee) {
> + dev_err(&bus->spmic->dev, "disallowed SPMI write to sid=%u, addr=0x%04X\n",
> sid, addr);
> return -EPERM;
> }
> @@ -1297,15 +1333,16 @@ static int pmic_arb_get_core_resources_v7(struct platform_device *pdev,
> * v7 offset per ee and per apid for observer channels and per apid for
> * read/write channels.
> */
> -static int pmic_arb_offset_v7(struct spmi_pmic_arb *pmic_arb, u8 sid, u16 addr,
> - enum pmic_arb_channel ch_type)
> +static int pmic_arb_offset_v7(struct spmi_pmic_arb_bus *bus, u8 sid, u16 addr,
> + enum pmic_arb_channel ch_type)
> {
> + struct spmi_pmic_arb *pmic_arb = bus->pmic_arb;
> u16 apid;
> int rc;
> u32 offset = 0;
> u16 ppid = (sid << 8) | (addr >> 8);
>
> - rc = pmic_arb->ver_ops->ppid_to_apid(pmic_arb, ppid);
> + rc = pmic_arb->ver_ops->ppid_to_apid(bus, ppid);
> if (rc < 0)
> return rc;
>
> @@ -1315,8 +1352,8 @@ static int pmic_arb_offset_v7(struct spmi_pmic_arb *pmic_arb, u8 sid, u16 addr,
> offset = 0x8000 * pmic_arb->ee + 0x20 * apid;
> break;
> case PMIC_ARB_CHANNEL_RW:
> - if (pmic_arb->apid_data[apid].write_ee != pmic_arb->ee) {
> - dev_err(&pmic_arb->spmic->dev, "disallowed SPMI write to sid=%u, addr=0x%04X\n",
> + if (bus->apid_data[apid].write_ee != pmic_arb->ee) {
> + dev_err(&bus->spmic->dev, "disallowed SPMI write to sid=%u, addr=0x%04X\n",
> sid, addr);
> return -EPERM;
> }
> @@ -1338,104 +1375,110 @@ static u32 pmic_arb_fmt_cmd_v2(u8 opc, u8 sid, u16 addr, u8 bc)
> }
>
> static void __iomem *
> -pmic_arb_owner_acc_status_v1(struct spmi_pmic_arb *pmic_arb, u8 m, u16 n)
> +pmic_arb_owner_acc_status_v1(struct spmi_pmic_arb_bus *bus, u8 m, u16 n)
> {
> - return pmic_arb->intr + 0x20 * m + 0x4 * n;
> + return bus->intr + 0x20 * m + 0x4 * n;
> }
>
> static void __iomem *
> -pmic_arb_owner_acc_status_v2(struct spmi_pmic_arb *pmic_arb, u8 m, u16 n)
> +pmic_arb_owner_acc_status_v2(struct spmi_pmic_arb_bus *bus, u8 m, u16 n)
> {
> - return pmic_arb->intr + 0x100000 + 0x1000 * m + 0x4 * n;
> + return bus->intr + 0x100000 + 0x1000 * m + 0x4 * n;
> }
>
> static void __iomem *
> -pmic_arb_owner_acc_status_v3(struct spmi_pmic_arb *pmic_arb, u8 m, u16 n)
> +pmic_arb_owner_acc_status_v3(struct spmi_pmic_arb_bus *bus, u8 m, u16 n)
> {
> - return pmic_arb->intr + 0x200000 + 0x1000 * m + 0x4 * n;
> + return bus->intr + 0x200000 + 0x1000 * m + 0x4 * n;
> }
>
> static void __iomem *
> -pmic_arb_owner_acc_status_v5(struct spmi_pmic_arb *pmic_arb, u8 m, u16 n)
> +pmic_arb_owner_acc_status_v5(struct spmi_pmic_arb_bus *bus, u8 m, u16 n)
> {
> - return pmic_arb->intr + 0x10000 * m + 0x4 * n;
> + return bus->intr + 0x10000 * m + 0x4 * n;
> }
>
> static void __iomem *
> -pmic_arb_owner_acc_status_v7(struct spmi_pmic_arb *pmic_arb, u8 m, u16 n)
> +pmic_arb_owner_acc_status_v7(struct spmi_pmic_arb_bus *bus, u8 m, u16 n)
> {
> - return pmic_arb->intr + 0x1000 * m + 0x4 * n;
> + return bus->intr + 0x1000 * m + 0x4 * n;
> }
>
> static void __iomem *
> -pmic_arb_acc_enable_v1(struct spmi_pmic_arb *pmic_arb, u16 n)
> +pmic_arb_acc_enable_v1(struct spmi_pmic_arb_bus *bus, u16 n)
> {
> - return pmic_arb->intr + 0x200 + 0x4 * n;
> + return bus->intr + 0x200 + 0x4 * n;
> }
>
> static void __iomem *
> -pmic_arb_acc_enable_v2(struct spmi_pmic_arb *pmic_arb, u16 n)
> +pmic_arb_acc_enable_v2(struct spmi_pmic_arb_bus *bus, u16 n)
> {
> - return pmic_arb->intr + 0x1000 * n;
> + return bus->intr + 0x1000 * n;
> }
>
> static void __iomem *
> -pmic_arb_acc_enable_v5(struct spmi_pmic_arb *pmic_arb, u16 n)
> +pmic_arb_acc_enable_v5(struct spmi_pmic_arb_bus *bus, u16 n)
> {
> + struct spmi_pmic_arb *pmic_arb = bus->pmic_arb;
> return pmic_arb->wr_base + 0x100 + 0x10000 * n;
> }
>
> static void __iomem *
> -pmic_arb_acc_enable_v7(struct spmi_pmic_arb *pmic_arb, u16 n)
> +pmic_arb_acc_enable_v7(struct spmi_pmic_arb_bus *bus, u16 n)
> {
> + struct spmi_pmic_arb *pmic_arb = bus->pmic_arb;
> return pmic_arb->wr_base + 0x100 + 0x1000 * n;
> }
>
> static void __iomem *
> -pmic_arb_irq_status_v1(struct spmi_pmic_arb *pmic_arb, u16 n)
> +pmic_arb_irq_status_v1(struct spmi_pmic_arb_bus *bus, u16 n)
> {
> - return pmic_arb->intr + 0x600 + 0x4 * n;
> + return bus->intr + 0x600 + 0x4 * n;
> }
>
> static void __iomem *
> -pmic_arb_irq_status_v2(struct spmi_pmic_arb *pmic_arb, u16 n)
> +pmic_arb_irq_status_v2(struct spmi_pmic_arb_bus *bus, u16 n)
> {
> - return pmic_arb->intr + 0x4 + 0x1000 * n;
> + return bus->intr + 0x4 + 0x1000 * n;
> }
>
> static void __iomem *
> -pmic_arb_irq_status_v5(struct spmi_pmic_arb *pmic_arb, u16 n)
> +pmic_arb_irq_status_v5(struct spmi_pmic_arb_bus *bus, u16 n)
> {
> + struct spmi_pmic_arb *pmic_arb = bus->pmic_arb;
> return pmic_arb->wr_base + 0x104 + 0x10000 * n;
> }
>
> static void __iomem *
> -pmic_arb_irq_status_v7(struct spmi_pmic_arb *pmic_arb, u16 n)
> +pmic_arb_irq_status_v7(struct spmi_pmic_arb_bus *bus, u16 n)
> {
> + struct spmi_pmic_arb *pmic_arb = bus->pmic_arb;
> return pmic_arb->wr_base + 0x104 + 0x1000 * n;
> }
>
> static void __iomem *
> -pmic_arb_irq_clear_v1(struct spmi_pmic_arb *pmic_arb, u16 n)
> +pmic_arb_irq_clear_v1(struct spmi_pmic_arb_bus *bus, u16 n)
> {
> - return pmic_arb->intr + 0xA00 + 0x4 * n;
> + return bus->intr + 0xA00 + 0x4 * n;
> }
>
> static void __iomem *
> -pmic_arb_irq_clear_v2(struct spmi_pmic_arb *pmic_arb, u16 n)
> +pmic_arb_irq_clear_v2(struct spmi_pmic_arb_bus *bus, u16 n)
> {
> - return pmic_arb->intr + 0x8 + 0x1000 * n;
> + return bus->intr + 0x8 + 0x1000 * n;
> }
>
> static void __iomem *
> -pmic_arb_irq_clear_v5(struct spmi_pmic_arb *pmic_arb, u16 n)
> +pmic_arb_irq_clear_v5(struct spmi_pmic_arb_bus *bus, u16 n)
> {
> + struct spmi_pmic_arb *pmic_arb = bus->pmic_arb;
> return pmic_arb->wr_base + 0x108 + 0x10000 * n;
> }
>
> static void __iomem *
> -pmic_arb_irq_clear_v7(struct spmi_pmic_arb *pmic_arb, u16 n)
> +pmic_arb_irq_clear_v7(struct spmi_pmic_arb_bus *bus, u16 n)
> {
> + struct spmi_pmic_arb *pmic_arb = bus->pmic_arb;
> return pmic_arb->wr_base + 0x108 + 0x1000 * n;
> }
>
> @@ -1455,9 +1498,9 @@ static u32 pmic_arb_apid_map_offset_v7(u16 n)
> }
>
> static void __iomem *
> -pmic_arb_apid_owner_v2(struct spmi_pmic_arb *pmic_arb, u16 n)
> +pmic_arb_apid_owner_v2(struct spmi_pmic_arb_bus *bus, u16 n)
> {
> - return pmic_arb->cnfg + 0x700 + 0x4 * n;
> + return bus->cnfg + 0x700 + 0x4 * n;
> }
>
> /*
> @@ -1466,9 +1509,9 @@ pmic_arb_apid_owner_v2(struct spmi_pmic_arb *pmic_arb, u16 n)
> * 0.
> */
> static void __iomem *
> -pmic_arb_apid_owner_v7(struct spmi_pmic_arb *pmic_arb, u16 n)
> +pmic_arb_apid_owner_v7(struct spmi_pmic_arb_bus *bus, u16 n)
> {
> - return pmic_arb->cnfg + 0x4 * (n - pmic_arb->base_apid);
> + return bus->cnfg + 0x4 * (n - bus->base_apid);
> }
>
> static const struct pmic_arb_ver_ops pmic_arb_v1 = {
> @@ -1558,29 +1601,120 @@ static const struct irq_domain_ops pmic_arb_irq_domain_ops = {
> .translate = qpnpint_irq_domain_translate,
> };
>
> +static int spmi_pmic_arb_bus_init(struct platform_device *pdev,
> + struct device_node *node,
> + struct spmi_pmic_arb *pmic_arb)
> +{
> + struct spmi_pmic_arb_bus *bus;
> + struct device *dev = &pdev->dev;
> + struct spmi_controller *ctrl;
> + void __iomem *intr;
> + void __iomem *cnfg;
> + int index, ret;
> + u32 irq;
> +
> + ctrl = devm_spmi_controller_alloc(dev, sizeof(*bus));
> + if (IS_ERR(ctrl))
> + return PTR_ERR(ctrl);
> +
> + ctrl->cmd = pmic_arb_cmd;
> + ctrl->read_cmd = pmic_arb_read_cmd;
> + ctrl->write_cmd = pmic_arb_write_cmd;
> +
> + bus = spmi_controller_get_drvdata(ctrl);
> +
> + pmic_arb->bus = bus;
> +
> + bus->ppid_to_apid = devm_kcalloc(dev, PMIC_ARB_MAX_PPID,
> + sizeof(*bus->ppid_to_apid),
> + GFP_KERNEL);
> + if (!bus->ppid_to_apid)
> + return -ENOMEM;
> +
> + bus->apid_data = devm_kcalloc(dev, pmic_arb->max_periphs,
> + sizeof(*bus->apid_data),
> + GFP_KERNEL);
> + if (!bus->apid_data)
> + return -ENOMEM;
> +
> + index = of_property_match_string(node, "reg-names", "cnfg");
> + if (index < 0) {
> + dev_err(dev, "cnfg reg region missing");
> + return -EINVAL;
> + }
> +
> + cnfg = devm_of_iomap(dev, node, index, NULL);
> + if (IS_ERR(cnfg))
> + return PTR_ERR(cnfg);
> +
> + index = of_property_match_string(node, "reg-names", "intr");
> + if (index < 0) {
> + dev_err(dev, "intr reg region missing");
> + return -EINVAL;
> + }
> +
> + intr = devm_of_iomap(dev, node, index, NULL);
> + if (IS_ERR(intr))
> + return PTR_ERR(intr);
> +
> + irq = of_irq_get_byname(node, "periph_irq");
> + if (irq < 0)
> + return irq;
> +
> + bus->pmic_arb = pmic_arb;
> + bus->intr = intr;
> + bus->cnfg = cnfg;
> + bus->irq = irq;
> + bus->spmic = ctrl;
> +
> + ret = pmic_arb->ver_ops->init_apid(bus);
> + if (ret)
> + return ret;
> +
> + dev_dbg(&pdev->dev, "adding irq domain\n");
> +
> + bus->domain = irq_domain_add_tree(dev->of_node,
> + &pmic_arb_irq_domain_ops, bus);
> + if (!bus->domain) {
> + dev_err(&pdev->dev, "unable to create irq_domain\n");
> + return -ENOMEM;
> + }
> +
> + irq_set_chained_handler_and_data(bus->irq,
> + pmic_arb_chained_irq, bus);
> +
> + ctrl->dev.of_node = node;
> +
> + ret = devm_spmi_controller_add(dev, ctrl);
> + if (ret)
> + return ret;
> +
> + return 0;
> +}
> +
> static int spmi_pmic_arb_probe(struct platform_device *pdev)
> {
> struct spmi_pmic_arb *pmic_arb;
> - struct spmi_controller *ctrl;
> + struct device *dev = &pdev->dev;
> struct resource *res;
> void __iomem *core;
> u32 channel, ee, hw_ver;
> int err;
>
> - ctrl = devm_spmi_controller_alloc(&pdev->dev, sizeof(*pmic_arb));
> - if (IS_ERR(ctrl))
> - return PTR_ERR(ctrl);
> -
> - pmic_arb = spmi_controller_get_drvdata(ctrl);
> - pmic_arb->spmic = ctrl;
> + pmic_arb = devm_kzalloc(dev, sizeof(*pmic_arb), GFP_KERNEL);
> + if (!pmic_arb)
> + return -ENOMEM;
>
> res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "core");
> - core = devm_ioremap(&ctrl->dev, res->start, resource_size(res));
> + core = devm_ioremap(dev, res->start, resource_size(res));
> if (IS_ERR(core))
> return PTR_ERR(core);
>
> pmic_arb->core_size = resource_size(res);
>
> + platform_set_drvdata(pdev, pmic_arb);
> + raw_spin_lock_init(&pmic_arb->lock);
> +
> hw_ver = readl_relaxed(core + PMIC_ARB_VERSION);
>
> if (hw_ver < PMIC_ARB_VERSION_V2_MIN)
> @@ -1594,30 +1728,12 @@ static int spmi_pmic_arb_probe(struct platform_device *pdev)
> else
> pmic_arb->ver_ops = &pmic_arb_v7;
>
> - dev_info(&ctrl->dev, "PMIC arbiter version %s (0x%x)\n",
> - pmic_arb->ver_ops->ver_str, hw_ver);
> -
> err = pmic_arb->ver_ops->get_core_resources(pdev, core);
> if (err)
> return err;
>
> - err = pmic_arb->ver_ops->init_apid(pmic_arb);
> - if (err)
> - return err;
> -
> - res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "intr");
> - pmic_arb->intr = devm_ioremap_resource(&ctrl->dev, res);
> - if (IS_ERR(pmic_arb->intr))
> - return PTR_ERR(pmic_arb->intr);
> -
> - res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "cnfg");
> - pmic_arb->cnfg = devm_ioremap_resource(&ctrl->dev, res);
> - if (IS_ERR(pmic_arb->cnfg))
> - return PTR_ERR(pmic_arb->cnfg);
> -
> - pmic_arb->irq = platform_get_irq_byname(pdev, "periph_irq");
> - if (pmic_arb->irq < 0)
> - return pmic_arb->irq;
> + dev_info(dev, "PMIC arbiter version %s (0x%x)\n",
> + pmic_arb->ver_ops->ver_str, hw_ver);
>
> err = of_property_read_u32(pdev->dev.of_node, "qcom,channel", &channel);
> if (err) {
> @@ -1646,42 +1762,17 @@ static int spmi_pmic_arb_probe(struct platform_device *pdev)
>
> pmic_arb->ee = ee;
>
> - platform_set_drvdata(pdev, ctrl);
> - raw_spin_lock_init(&pmic_arb->lock);
> -
> - ctrl->cmd = pmic_arb_cmd;
> - ctrl->read_cmd = pmic_arb_read_cmd;
> - ctrl->write_cmd = pmic_arb_write_cmd;
> -
> - dev_dbg(&pdev->dev, "adding irq domain\n");
> - pmic_arb->domain = irq_domain_add_tree(pdev->dev.of_node,
> - &pmic_arb_irq_domain_ops, pmic_arb);
> - if (!pmic_arb->domain) {
> - dev_err(&pdev->dev, "unable to create irq_domain\n");
> - return -ENOMEM;
> - }
> -
> - irq_set_chained_handler_and_data(pmic_arb->irq, pmic_arb_chained_irq,
> - pmic_arb);
> - err = spmi_controller_add(ctrl);
> - if (err)
> - goto err_domain_remove;
> -
> - return 0;
> -
> -err_domain_remove:
> - irq_set_chained_handler_and_data(pmic_arb->irq, NULL, NULL);
> - irq_domain_remove(pmic_arb->domain);
> - return err;
> + return spmi_pmic_arb_bus_init(pdev, dev->of_node, pmic_arb);
> }
>
> static void spmi_pmic_arb_remove(struct platform_device *pdev)
> {
> - struct spmi_controller *ctrl = platform_get_drvdata(pdev);
> - struct spmi_pmic_arb *pmic_arb = spmi_controller_get_drvdata(ctrl);
> - spmi_controller_remove(ctrl);
> - irq_set_chained_handler_and_data(pmic_arb->irq, NULL, NULL);
> - irq_domain_remove(pmic_arb->domain);
> + struct spmi_pmic_arb *pmic_arb = platform_get_drvdata(pdev);
> + struct spmi_pmic_arb_bus *bus = pmic_arb->bus;
> +
> + irq_set_chained_handler_and_data(bus->irq,
> + NULL, NULL);
> + irq_domain_remove(bus->domain);
> }
>
> static const struct of_device_id spmi_pmic_arb_match_table[] = {
>
Reviewed-by: Neil Armstrong <neil.armstrong@linaro.org>
^ permalink raw reply
* Re: [PATCH] arm64: dts: ti: k3-j722s: Disable ethernet ports by default
From: Michael Walle @ 2024-04-03 7:35 UTC (permalink / raw)
To: Francesco Dolcini
Cc: Nishanth Menon, Vignesh Raghavendra, Tero Kristo, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, linux-arm-kernel, devicetree,
linux-kernel
In-Reply-To: <20240402165824.GA32125@francesco-nb>
[-- Attachment #1: Type: text/plain, Size: 1881 bytes --]
Hi Francesco,
On Tue Apr 2, 2024 at 6:58 PM CEST, Francesco Dolcini wrote:
> On Tue, Apr 02, 2024 at 05:18:02PM +0200, Michael Walle wrote:
> > Device tree best practice is to disable any external interface in the
> > dtsi and just enable them if needed in the device tree. Thus, disable
> > both ethernet ports by default and just enable the one used by the EVM
> > in its device tree.
> >
> > There is no functional change.
> >
> > Signed-off-by: Michael Walle <mwalle@kernel.org>
> > ---
> > This should also be true for all the other SoCs. But I don't wanted to
> > touch all the (older) device trees. j722s is pretty new, so there we
> > should get it right.
> > ---
> > arch/arm64/boot/dts/ti/k3-j722s-evm.dts | 5 +----
> > arch/arm64/boot/dts/ti/k3-j722s.dtsi | 8 ++++++++
> > 2 files changed, 9 insertions(+), 4 deletions(-)
> >
> > diff --git a/arch/arm64/boot/dts/ti/k3-j722s-evm.dts b/arch/arm64/boot/dts/ti/k3-j722s-evm.dts
> > index d045dc7dde0c..afe7f68e6a4b 100644
> > --- a/arch/arm64/boot/dts/ti/k3-j722s-evm.dts
> > +++ b/arch/arm64/boot/dts/ti/k3-j722s-evm.dts
> > @@ -224,14 +224,11 @@ cpsw3g_phy0: ethernet-phy@0 {
> > };
> >
> > &cpsw_port1 {
> > + status = "okay";
>
> status should be the last property, according to the dts coding guidelines.
Thanks for pointing that out. There is
devicetree/bindings/dts-coding-style.rst, which is in fact new to
me. Up until now, I was under the impression that how this is
handled is up to the maintainer of the SoC. I know that for the NXP
Layerscape for example, the maintainer will have an eye esp. for
that. But here it seems kinda random/all over the place. That being
said, I tried to be consistent with the other cpsw* nodes.
Anyway, I'll change it to come last.
> > phy-mode = "rgmii-rxid";
> > phy-handle = <&cpsw3g_phy0>;
> > };
-michael
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 297 bytes --]
^ permalink raw reply
* Re: [PATCH 5/5] arm64: dts: Add device tree source for the Au-Zone Maivin Starter Kit
From: Laurent Pinchart @ 2024-04-03 7:35 UTC (permalink / raw)
To: Francesco Dolcini
Cc: Shawn Guo, devicetree, imx, linux-arm-kernel, Trevor Zaharichuk,
Greg Lytle, Shawn Guo, Sascha Hauer, Pengutronix Kernel Team,
Fabio Estevam, Rob Herring, Krzysztof Kozlowski, Conor Dooley
In-Reply-To: <20240403070651.GB5070@francesco-nb>
On Wed, Apr 03, 2024 at 09:06:51AM +0200, Francesco Dolcini wrote:
> Hello Laurent,
>
> On Wed, Apr 03, 2024 at 08:30:11AM +0800, Shawn Guo wrote:
> > On Mon, Mar 25, 2024 at 10:32:45PM +0200, Laurent Pinchart wrote:
> > > diff --git a/arch/arm64/boot/dts/freescale/imx8mp-maivin.dts b/arch/arm64/boot/dts/freescale/imx8mp-maivin.dts
> > > new file mode 100644
> > > index 000000000000..2d1c8e782465
> > > --- /dev/null
> > > +++ b/arch/arm64/boot/dts/freescale/imx8mp-maivin.dts
> > > @@ -0,0 +1,236 @@
>
> [...]
>
> > > +/* Verdin I2C_2_DSI */
> > > +&i2c2 {
> > > + status = "okay";
> > > +
> > > + clock-frequency = <400000>;
> > > + scl-gpios = <&gpio5 16 (GPIO_ACTIVE_HIGH | GPIO_OPEN_DRAIN)>;
> > > + sda-gpios = <&gpio5 17 (GPIO_ACTIVE_HIGH | GPIO_OPEN_DRAIN)>;
> >
> > We usually end property list with 'status'.
>
> This is now a written and explicit guideline, no longer tribal knowledge,
> see https://docs.kernel.org/devicetree/bindings/dts-coding-style.html#order-of-properties-in-device-node
Thanks.
Any chance to teach checkpatch.pl (and/or the DT checker) about that ? :-)
--
Regards,
Laurent Pinchart
^ permalink raw reply
* Re: [PATCH v8 3/7] spmi: pmic-arb: Fix some compile warnings about members not being described
From: Neil Armstrong @ 2024-04-03 7:35 UTC (permalink / raw)
To: Abel Vesa, Stephen Boyd, Matthias Brugger, Bjorn Andersson,
Konrad Dybcio, Dmitry Baryshkov, AngeloGioacchino Del Regno,
Rob Herring, Krzysztof Kozlowski, Conor Dooley
Cc: Srini Kandagatla, Johan Hovold, linux-kernel, linux-arm-kernel,
linux-arm-msm, linux-mediatek, devicetree
In-Reply-To: <20240402-spmi-multi-master-support-v8-3-ce6f2d14a058@linaro.org>
On 02/04/2024 14:07, Abel Vesa wrote:
> Fix the following compile warnings:
>
> warning: Function parameter or struct member 'core' not described in 'spmi_pmic_arb'
> warning: Function parameter or struct member 'core_size' not described in 'spmi_pmic_arb'
> warning: Function parameter or struct member 'mapping_table_valid' not described in 'spmi_pmic_arb'
> warning: Function parameter or struct member 'pmic_arb' not described in 'pmic_arb_read_data'
> warning: Function parameter or struct member 'pmic_arb' not described in 'pmic_arb_write_data'
>
> Signed-off-by: Abel Vesa <abel.vesa@linaro.org>
> ---
> drivers/spmi/spmi-pmic-arb.c | 5 +++++
> 1 file changed, 5 insertions(+)
>
> diff --git a/drivers/spmi/spmi-pmic-arb.c b/drivers/spmi/spmi-pmic-arb.c
> index 9ed1180fe31f..704fd4506971 100644
> --- a/drivers/spmi/spmi-pmic-arb.c
> +++ b/drivers/spmi/spmi-pmic-arb.c
> @@ -132,6 +132,8 @@ struct apid_data {
> * @wr_base: on v1 "core", on v2 "chnls" register base off DT.
> * @intr: address of the SPMI interrupt control registers.
> * @cnfg: address of the PMIC Arbiter configuration registers.
> + * @core: core register base for v2 and above only (see above)
> + * @core_size: core register base size
> * @lock: lock to synchronize accesses.
> * @channel: execution environment channel to use for accesses.
> * @irq: PMIC ARB interrupt.
> @@ -144,6 +146,7 @@ struct apid_data {
> * @apid_count: on v5 and v7: number of APIDs associated with the
> * particular SPMI bus instance
> * @mapping_table: in-memory copy of PPID -> APID mapping table.
> + * @mapping_table_valid:bitmap containing valid-only periphs
> * @domain: irq domain object for PMIC IRQ domain
> * @spmic: SPMI controller object
> * @ver_ops: version dependent operations.
> @@ -232,6 +235,7 @@ static inline void pmic_arb_set_rd_cmd(struct spmi_pmic_arb *pmic_arb,
>
> /**
> * pmic_arb_read_data: reads pmic-arb's register and copy 1..4 bytes to buf
> + * @pmic_arb: the SPMI PMIC arbiter
> * @bc: byte count -1. range: 0..3
> * @reg: register's address
> * @buf: output parameter, length must be bc + 1
> @@ -246,6 +250,7 @@ pmic_arb_read_data(struct spmi_pmic_arb *pmic_arb, u8 *buf, u32 reg, u8 bc)
>
> /**
> * pmic_arb_write_data: write 1..4 bytes from buf to pmic-arb's register
> + * @pmic_arb: the SPMI PMIC arbiter
> * @bc: byte-count -1. range: 0..3.
> * @reg: register's address.
> * @buf: buffer to write. length must be bc + 1.
>
Reviewed-by: Neil Armstrong <neil.armstrong@linaro.org>
^ permalink raw reply
* Re: [PATCH v18 2/9] usb: dwc3: core: Access XHCI address space temporarily to read port info
From: Krishna Kurapati PSSNV @ 2024-04-03 7:33 UTC (permalink / raw)
To: Johan Hovold
Cc: Thinh Nguyen, Krzysztof Kozlowski, Rob Herring, Bjorn Andersson,
Wesley Cheng, Konrad Dybcio, Greg Kroah-Hartman, Conor Dooley,
Felipe Balbi, devicetree@vger.kernel.org,
linux-arm-msm@vger.kernel.org, linux-usb@vger.kernel.org,
linux-kernel@vger.kernel.org, quic_ppratap@quicinc.com,
quic_jackp@quicinc.com, Johan Hovold
In-Reply-To: <Zgz_3AspRRfYqOwZ@hovoldconsulting.com>
On 4/3/2024 12:36 PM, Johan Hovold wrote:
> On Wed, Apr 03, 2024 at 10:54:25AM +0530, Krishna Kurapati PSSNV wrote:
>
>>>> +static int dwc3_read_port_info(struct dwc3 *dwc)
>>>> +{
>>>> + void __iomem *base;
>>>> + u8 major_revision;
>>>> + u32 offset;
>>>> + u32 val;
>>>> +
>>>> + /*
>>>> + * Remap xHCI address space to access XHCI ext cap regs since it is
>>>> + * needed to get information on number of ports present.
>>>> + */
>>>> + base = ioremap(dwc->xhci_resources[0].start,
>>>> + resource_size(&dwc->xhci_resources[0]));
>>>> + if (IS_ERR(base))
>>>> + return PTR_ERR(base);
>>>
>>> Looks like you forgot to address some of the comments you said you'd
>>> update previously if you submit a new version to the series.
>>>
>>> [*] https://lore.kernel.org/linux-usb/af73110d-e13e-4183-af11-aed869ac0a31@quicinc.com/
>>>
>>
>> Apologies. I agree. I was too much focused on acpi removal and interrupt
>> cleanup, I forgot the last comment you gave.
>>
>> Can I send in a separate patch for this ?
>
> The series has not been merged yet so you can address both issues in a
> v19. Perhaps wait a day or two in case Thinh has further comments.
>
Sure Johan.
Also after making the following two changes:
1. Rename dwc3_read_port_info(...) to dwc3_get_num_ports(...)
2. Changing "if (IS_ERR(base))" to "if (!base)"
Can I still retain your RB tag ?
Regards,
Krishna,
^ permalink raw reply
* Re: [PATCH v3] clk: starfive: pll: Fix lower rate of CPUfreq by setting PLL0 rate to 1.5GHz
From: Krzysztof Kozlowski @ 2024-04-03 7:23 UTC (permalink / raw)
To: Xingyu Wu, Michael Turquette, Stephen Boyd, Conor Dooley,
Emil Renner Berthing, Rob Herring, Krzysztof Kozlowski
Cc: Paul Walmsley, Palmer Dabbelt, Albert Ou, Hal Feng,
linux-kernel@vger.kernel.org, linux-clk@vger.kernel.org,
linux-riscv@lists.infradead.org, devicetree@vger.kernel.org
In-Reply-To: <NTZPR01MB095662C4A6FCAEB7B35C48FF9F3DA@NTZPR01MB0956.CHNPR01.prod.partner.outlook.cn>
On 03/04/2024 09:19, Xingyu Wu wrote:
> On 03/04/2024 0:18, Krzysztof Kozlowski wrote:
>>
>> On 02/04/2024 11:09, Xingyu Wu wrote:
>>> CPUfreq supports 4 cpu frequency loads on 375/500/750/1500MHz.
>>> But now PLL0 rate is 1GHz and the cpu frequency loads become
>>> 333/500/500/1000MHz in fact.
>>>
>>> So PLL0 rate should be default set to 1.5GHz. But setting the
>>> PLL0 rate need certain steps:
>>>
>>> 1. Change the parent of cpu_root clock to OSC clock.
>>> 2. Change the divider of cpu_core if PLL0 rate is higher than
>>> 1.25GHz before CPUfreq boot.
>>> 3. Change the parent of cpu_root clock back to PLL0 clock.
>>>
>>> Reviewed-by: Hal Feng <hal.feng@starfivetech.com>
>>> Fixes: e2c510d6d630 ("riscv: dts: starfive: Add cpu scaling for JH7110
>>> SoC")
>>> Signed-off-by: Xingyu Wu <xingyu.wu@starfivetech.com>
>>> ---
>>>
>>> Hi Stephen and Emil,
>>>
>>> This patch fixes the issue about lower rate of CPUfreq[1] by setting
>>> PLL0 rate to 1.5GHz.
>>>
>>> In order not to affect the cpu operation, setting the PLL0 rate need
>>> certain steps. The cpu_root's parent clock should be changed first.
>>> And the divider of the cpu_core clock should be set to 2 so they won't
>>> crash when setting 1.5GHz without voltage regulation. Due to PLL
>>> driver boot earlier than SYSCRG driver, cpu_core and cpu_root clocks
>>> are using by ioremap().
>>>
>>> [1]: https://github.com/starfive-tech/VisionFive2/issues/55
>>>
>>> Previous patch link:
>>> v2:
>>> https://lore.kernel.org/all/20230821152915.208366-1-xingyu.wu@starfive
>>> tech.com/
>>> v1:
>>> https://lore.kernel.org/all/20230811033631.160912-1-xingyu.wu@starfive
>>> tech.com/
>>>
>>> Thanks,
>>> Xingyu Wu
>>> ---
>>> .../jh7110-starfive-visionfive-2.dtsi | 5 +
>>> .../clk/starfive/clk-starfive-jh7110-pll.c | 102 ++++++++++++++++++
>>
>> Please do not mix DTS and driver code. That's not really portable. DTS is being
>> exported and used in other projects.
>
> OK, I will submit that in two patches.
>
>>
>> ...
>>
>>>
>>> @@ -458,6 +535,8 @@ static int jh7110_pll_probe(struct platform_device
>> *pdev)
>>> struct jh7110_pll_priv *priv;
>>> unsigned int idx;
>>> int ret;
>>> + struct device_node *np;
>>> + struct resource res;
>>>
>>> priv = devm_kzalloc(&pdev->dev, sizeof(*priv), GFP_KERNEL);
>>> if (!priv)
>>> @@ -489,6 +568,29 @@ static int jh7110_pll_probe(struct platform_device
>> *pdev)
>>> return ret;
>>> }
>>>
>>> + priv->is_first_set = true;
>>> + np = of_find_compatible_node(NULL, NULL, "starfive,jh7110-syscrg");
>>
>> Your drivers should not do it. It's fragile, hides true link/dependency.
>> Please use phandles.
>>
>>
>>> + if (!np) {
>>> + ret = PTR_ERR(np);
>>> + dev_err(priv->dev, "failed to get syscrg node\n");
>>> + goto np_put;
>>> + }
>>> +
>>> + ret = of_address_to_resource(np, 0, &res);
>>> + if (ret) {
>>> + dev_err(priv->dev, "failed to get syscrg resource\n");
>>> + goto np_put;
>>> + }
>>> +
>>> + priv->syscrg_base = ioremap(res.start, resource_size(&res));
>>> + if (!priv->syscrg_base)
>>> + ret = -ENOMEM;
>>
>> Why are you mapping other device's IO? How are you going to ensure synced
>> access to registers?
>
> Because setting PLL0 rate need specific steps and use the clocks of SYSCRG.
That's not a reason to map other device's IO. That could be a reason for
having syscon or some other sort of relationship, like clock or reset.
> But SYSCRG driver also need PLL clock to be clock source when adding clock
> providers. I tried to add SYSCRG clocks in 'clocks' property in DT and use
> clk_get() to get the clocks. But it could not run and crash. So I use ioremap()
> instead.
So instead of properly model the relationship, you entangle the drivers
even more.
Please come with a proper design for this. I have no clue about your
hardware, but that looks like you are asynchronously configuring the
same hardware in two different places.
Sorry, that's poor code.
Best regards,
Krzysztof
^ permalink raw reply
* Re: [PATCH v1 1/5] dt-bindings: pwm: Add Loongson PWM controller
From: Binbin Zhou @ 2024-04-03 7:22 UTC (permalink / raw)
To: Krzysztof Kozlowski
Cc: Rob Herring, Binbin Zhou, Huacai Chen, Uwe Kleine-König,
Krzysztof Kozlowski, Conor Dooley, Huacai Chen, loongson-kernel,
linux-pwm, devicetree, Xuerui Wang, loongarch
In-Reply-To: <d0769eb1-984b-4e2c-8d9f-818113d8afb2@linaro.org>
On Wed, Apr 3, 2024 at 1:00 PM Krzysztof Kozlowski
<krzysztof.kozlowski@linaro.org> wrote:
>
> On 03/04/2024 04:37, Binbin Zhou wrote:
> > Hi Rob:
> >
> > Thanks for your reply.
> >
> > On Tue, Apr 2, 2024 at 11:40 PM Rob Herring <robh@kernel.org> wrote:
> >>
> >> On Tue, Apr 02, 2024 at 03:58:38PM +0800, Binbin Zhou wrote:
> >>> Add Loongson PWM controller binding with DT schema format using
> >>> json-schema.
> >>>
> >>> Signed-off-by: Binbin Zhou <zhoubinbin@loongson.cn>
> >>> ---
> >>> .../devicetree/bindings/pwm/pwm-loongson.yaml | 64 +++++++++++++++++++
> >>
> >> Filename should match compatible.
> >
> > Emm... How about renaming it as loongson, pwm.yaml?
>
> Use the fallback, so loongson,ls7a-pwm.yaml
Ok, I got it.
Thanks.
Binbin
>
> Best regards,
> Krzysztof
>
^ permalink raw reply
* RE: [PATCH v3] clk: starfive: pll: Fix lower rate of CPUfreq by setting PLL0 rate to 1.5GHz
From: Xingyu Wu @ 2024-04-03 7:19 UTC (permalink / raw)
To: Krzysztof Kozlowski, Michael Turquette, Stephen Boyd,
Conor Dooley, Emil Renner Berthing, Rob Herring,
Krzysztof Kozlowski
Cc: Paul Walmsley, Palmer Dabbelt, Albert Ou, Hal Feng,
linux-kernel@vger.kernel.org, linux-clk@vger.kernel.org,
linux-riscv@lists.infradead.org, devicetree@vger.kernel.org
In-Reply-To: <8d21b1bc-9402-41d4-bd81-c521c8a33d2d@kernel.org>
On 03/04/2024 0:18, Krzysztof Kozlowski wrote:
>
> On 02/04/2024 11:09, Xingyu Wu wrote:
> > CPUfreq supports 4 cpu frequency loads on 375/500/750/1500MHz.
> > But now PLL0 rate is 1GHz and the cpu frequency loads become
> > 333/500/500/1000MHz in fact.
> >
> > So PLL0 rate should be default set to 1.5GHz. But setting the
> > PLL0 rate need certain steps:
> >
> > 1. Change the parent of cpu_root clock to OSC clock.
> > 2. Change the divider of cpu_core if PLL0 rate is higher than
> > 1.25GHz before CPUfreq boot.
> > 3. Change the parent of cpu_root clock back to PLL0 clock.
> >
> > Reviewed-by: Hal Feng <hal.feng@starfivetech.com>
> > Fixes: e2c510d6d630 ("riscv: dts: starfive: Add cpu scaling for JH7110
> > SoC")
> > Signed-off-by: Xingyu Wu <xingyu.wu@starfivetech.com>
> > ---
> >
> > Hi Stephen and Emil,
> >
> > This patch fixes the issue about lower rate of CPUfreq[1] by setting
> > PLL0 rate to 1.5GHz.
> >
> > In order not to affect the cpu operation, setting the PLL0 rate need
> > certain steps. The cpu_root's parent clock should be changed first.
> > And the divider of the cpu_core clock should be set to 2 so they won't
> > crash when setting 1.5GHz without voltage regulation. Due to PLL
> > driver boot earlier than SYSCRG driver, cpu_core and cpu_root clocks
> > are using by ioremap().
> >
> > [1]: https://github.com/starfive-tech/VisionFive2/issues/55
> >
> > Previous patch link:
> > v2:
> > https://lore.kernel.org/all/20230821152915.208366-1-xingyu.wu@starfive
> > tech.com/
> > v1:
> > https://lore.kernel.org/all/20230811033631.160912-1-xingyu.wu@starfive
> > tech.com/
> >
> > Thanks,
> > Xingyu Wu
> > ---
> > .../jh7110-starfive-visionfive-2.dtsi | 5 +
> > .../clk/starfive/clk-starfive-jh7110-pll.c | 102 ++++++++++++++++++
>
> Please do not mix DTS and driver code. That's not really portable. DTS is being
> exported and used in other projects.
OK, I will submit that in two patches.
>
> ...
>
> >
> > @@ -458,6 +535,8 @@ static int jh7110_pll_probe(struct platform_device
> *pdev)
> > struct jh7110_pll_priv *priv;
> > unsigned int idx;
> > int ret;
> > + struct device_node *np;
> > + struct resource res;
> >
> > priv = devm_kzalloc(&pdev->dev, sizeof(*priv), GFP_KERNEL);
> > if (!priv)
> > @@ -489,6 +568,29 @@ static int jh7110_pll_probe(struct platform_device
> *pdev)
> > return ret;
> > }
> >
> > + priv->is_first_set = true;
> > + np = of_find_compatible_node(NULL, NULL, "starfive,jh7110-syscrg");
>
> Your drivers should not do it. It's fragile, hides true link/dependency.
> Please use phandles.
>
>
> > + if (!np) {
> > + ret = PTR_ERR(np);
> > + dev_err(priv->dev, "failed to get syscrg node\n");
> > + goto np_put;
> > + }
> > +
> > + ret = of_address_to_resource(np, 0, &res);
> > + if (ret) {
> > + dev_err(priv->dev, "failed to get syscrg resource\n");
> > + goto np_put;
> > + }
> > +
> > + priv->syscrg_base = ioremap(res.start, resource_size(&res));
> > + if (!priv->syscrg_base)
> > + ret = -ENOMEM;
>
> Why are you mapping other device's IO? How are you going to ensure synced
> access to registers?
Because setting PLL0 rate need specific steps and use the clocks of SYSCRG.
But SYSCRG driver also need PLL clock to be clock source when adding clock
providers. I tried to add SYSCRG clocks in 'clocks' property in DT and use
clk_get() to get the clocks. But it could not run and crash. So I use ioremap()
instead.
Best regards,
Xingyu Wu
^ permalink raw reply
* Re: [PATCH V2 RESEND 6/6] arm64: dts: qcom: sm8650: Add video and camera clock controllers
From: Jagadeesh Kona @ 2024-04-03 7:16 UTC (permalink / raw)
To: Dmitry Baryshkov
Cc: Bjorn Andersson, Konrad Dybcio, Michael Turquette, Stephen Boyd,
Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Vladimir Zapolskiy, linux-arm-msm, linux-clk, devicetree,
linux-kernel, Taniya Das, Satya Priya Kakitapalli, Ajit Pandey,
Imran Shaik
In-Reply-To: <008d574f-9c9e-48c6-b64e-89fb469cbde4@quicinc.com>
On 3/25/2024 11:38 AM, Jagadeesh Kona wrote:
>
>
> On 3/21/2024 6:43 PM, Dmitry Baryshkov wrote:
>> On Thu, 21 Mar 2024 at 11:27, Jagadeesh Kona <quic_jkona@quicinc.com>
>> wrote:
>>>
>>> Add device nodes for video and camera clock controllers on Qualcomm
>>> SM8650 platform.
>>>
>>> Signed-off-by: Jagadeesh Kona <quic_jkona@quicinc.com>
>>> ---
>>> arch/arm64/boot/dts/qcom/sm8650.dtsi | 28 ++++++++++++++++++++++++++++
>>> 1 file changed, 28 insertions(+)
>>>
>>> diff --git a/arch/arm64/boot/dts/qcom/sm8650.dtsi
>>> b/arch/arm64/boot/dts/qcom/sm8650.dtsi
>>> index 32c0a7b9aded..d862aa6be824 100644
>>> --- a/arch/arm64/boot/dts/qcom/sm8650.dtsi
>>> +++ b/arch/arm64/boot/dts/qcom/sm8650.dtsi
>>> @@ -4,6 +4,8 @@
>>> */
>>>
>>> #include <dt-bindings/clock/qcom,rpmh.h>
>>> +#include <dt-bindings/clock/qcom,sm8450-videocc.h>
>>> +#include <dt-bindings/clock/qcom,sm8650-camcc.h>
>>> #include <dt-bindings/clock/qcom,sm8650-dispcc.h>
>>> #include <dt-bindings/clock/qcom,sm8650-gcc.h>
>>> #include <dt-bindings/clock/qcom,sm8650-gpucc.h>
>>> @@ -3110,6 +3112,32 @@ opp-202000000 {
>>> };
>>> };
>>>
>>> + videocc: clock-controller@aaf0000 {
>>> + compatible = "qcom,sm8650-videocc";
>>> + reg = <0 0x0aaf0000 0 0x10000>;
>>> + clocks = <&bi_tcxo_div2>,
>>> + <&gcc GCC_VIDEO_AHB_CLK>;
>>> + power-domains = <&rpmhpd RPMHPD_MMCX>;
>>> + required-opps = <&rpmhpd_opp_low_svs>;
>>
>> The required-opps should no longer be necessary.
>>
>
> Sure, will check and remove this if not required.
I checked further on this and without required-opps, if there is no vote
on the power-domain & its peer from any other consumers, when runtime
get is called on device, it enables the power domain just at the minimum
non-zero level. But in some cases, the minimum non-zero level of
power-domain could be just retention and is not sufficient for clock
controller to operate, hence required-opps property is needed to specify
the minimum level required on power-domain for this clock controller.
Thanks,
Jagadeesh
>
>>> + #clock-cells = <1>;
>>> + #reset-cells = <1>;
>>> + #power-domain-cells = <1>;
>>> + };
>>> +
>>> + camcc: clock-controller@ade0000 {
>>> + compatible = "qcom,sm8650-camcc";
>>> + reg = <0 0x0ade0000 0 0x20000>;
>>> + clocks = <&gcc GCC_CAMERA_AHB_CLK>,
>>> + <&bi_tcxo_div2>,
>>> + <&bi_tcxo_ao_div2>,
>>> + <&sleep_clk>;
>>> + power-domains = <&rpmhpd RPMHPD_MMCX>;
>>> + required-opps = <&rpmhpd_opp_low_svs>;
>>> + #clock-cells = <1>;
>>> + #reset-cells = <1>;
>>> + #power-domain-cells = <1>;
>>> + };
>>> +
>>> mdss: display-subsystem@ae00000 {
>>> compatible = "qcom,sm8650-mdss";
>>> reg = <0 0x0ae00000 0 0x1000>;
>>> --
>>> 2.43.0
>>>
>>>
>>
>>
^ permalink raw reply
* Re: [PATCH 5/7] dt-bindings: phy: qcom,ipq8074-qmp-pcie: add ipq9574 gen3x2 PHY
From: Krzysztof Kozlowski @ 2024-04-03 7:15 UTC (permalink / raw)
To: Alexandru Gagniuc, Bjorn Andersson, Konrad Dybcio, Vinod Koul,
Kishon Vijay Abraham I, Rob Herring, Krzysztof Kozlowski,
Conor Dooley
Cc: ansuelsmth, robimarko, linux-arm-msm, linux-phy, devicetree,
linux-kernel
In-Reply-To: <20240402192555.1955204-5-mr.nuke.me@gmail.com>
On 02/04/2024 21:25, Alexandru Gagniuc wrote:
> The IPQ9574 gen3x2 PHY is very similar to IPQ6018. It requires two
> extra clocks named "anoc" and "snoc". Document this, and add a
> new compatible string for this PHY.
>
> Signed-off-by: Alexandru Gagniuc <mr.nuke.me@gmail.com>
> ---
> .../phy/qcom,ipq8074-qmp-pcie-phy.yaml | 47 +++++++++++++++++--
> 1 file changed, 42 insertions(+), 5 deletions(-)
>
> diff --git a/Documentation/devicetree/bindings/phy/qcom,ipq8074-qmp-pcie-phy.yaml b/Documentation/devicetree/bindings/phy/qcom,ipq8074-qmp-pcie-phy.yaml
> index 634cec5d57ea..b0dbd2726acd 100644
> --- a/Documentation/devicetree/bindings/phy/qcom,ipq8074-qmp-pcie-phy.yaml
> +++ b/Documentation/devicetree/bindings/phy/qcom,ipq8074-qmp-pcie-phy.yaml
> @@ -19,19 +19,19 @@ properties:
> - qcom,ipq6018-qmp-pcie-phy
> - qcom,ipq8074-qmp-gen3-pcie-phy
> - qcom,ipq8074-qmp-pcie-phy
> + - qcom,ipq9574-qmp-gen3x2-pcie-phy
>
> reg:
> items:
> - description: serdes
>
> clocks:
> - maxItems: 3
> + minItems: 3
> + maxItems: 5
>
> clock-names:
> - items:
> - - const: aux
> - - const: cfg_ahb
> - - const: pipe
> + minItems: 3
> + maxItems: 5
Just grow the list here to match ipq9574 and keep minItems: 3
>
> resets:
> maxItems: 2
> @@ -61,6 +61,43 @@ required:
> - clock-output-names
> - "#phy-cells"
>
> +allOf:
> + - if:
> + properties:
> + compatible:
> + contains:
> + enum:
> + - qcom,ipq6018-qmp-pcie-phy
> + - qcom,ipq8074-qmp-gen3-pcie-phy
> + - qcom,ipq8074-qmp-pcie-phy
> + then:
> + properties:
> + clocks:
> + maxItems: 3
> + clock-names:
> + items:
> + - const: aux
> + - const: cfg_ahb
> + - const: pipe
Only maxItems: 3
> +
> + - if:
> + properties:
> + compatible:
> + contains:
> + enum:
> + - qcom,ipq9574-qmp-gen3x2-pcie-phy
> + then:
> + properties:
> + clocks:
> + maxItems: 5
> + clock-names:
> + items:
> + - const: aux
> + - const: cfg_ahb
> + - const: pipe
> + - const: anoc
> + - const: snoc
minItems: 5
Best regards,
Krzysztof
^ permalink raw reply
* Re: [PATCH 3/7] dt-bindings: PCI: qcom: Add IPQ9574 PCIe controller
From: Krzysztof Kozlowski @ 2024-04-03 7:14 UTC (permalink / raw)
To: Alexandru Gagniuc, Bjorn Andersson, Konrad Dybcio, Bjorn Helgaas,
Lorenzo Pieralisi, Krzysztof Wilczyński, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Manivannan Sadhasivam
Cc: ansuelsmth, robimarko, linux-arm-msm, linux-pci, devicetree,
linux-kernel
In-Reply-To: <20240402192555.1955204-3-mr.nuke.me@gmail.com>
On 02/04/2024 21:25, Alexandru Gagniuc wrote:
> IPQ9574 has PCIe controllers which are almost identical to IPQ6018.
> The only difference is that the "iface" clock is not required.
> Document this difference along with the compatible string.
>
> Signed-off-by: Alexandru Gagniuc <mr.nuke.me@gmail.com>
> ---
> .../devicetree/bindings/pci/qcom,pcie.yaml | 32 +++++++++++++++++++
> 1 file changed, 32 insertions(+)
>
> diff --git a/Documentation/devicetree/bindings/pci/qcom,pcie.yaml b/Documentation/devicetree/bindings/pci/qcom,pcie.yaml
> index cf9a6910b542..6eb29547c18e 100644
> --- a/Documentation/devicetree/bindings/pci/qcom,pcie.yaml
> +++ b/Documentation/devicetree/bindings/pci/qcom,pcie.yaml
> @@ -26,6 +26,7 @@ properties:
> - qcom,pcie-ipq8064-v2
> - qcom,pcie-ipq8074
> - qcom,pcie-ipq8074-gen3
> + - qcom,pcie-ipq9574
> - qcom,pcie-msm8996
> - qcom,pcie-qcs404
> - qcom,pcie-sdm845
> @@ -383,6 +384,35 @@ allOf:
> - const: axi_s # AXI Slave clock
> - const: axi_bridge # AXI bridge clock
> - const: rchng
> +
> + - if:
> + properties:
> + compatible:
> + contains:
> + enum:
> + - qcom,pcie-ipq9574
> + then:
> + properties:
> + clocks:
> + minItems: 4
> + maxItems: 4
> + clock-names:
> + items:
> + - const: axi_m # AXI Master clock
> + - const: axi_s # AXI Slave clock
> + - const: axi_bridge # AXI bridge clock
> + - const: rchng
> +
> + - if:
> + properties:
> + compatible:
> + contains:
> + enum:
> + - qcom,pcie-ipq6018
> + - qcom,pcie-ipq8074-gen3
> + - qcom,pcie-ipq9574
> + then:
Do not introduce inconsistent style. All if:then: define both clocks and
resets, right? And after your patch not anymore?
Best regards,
Krzysztof
^ permalink raw reply
* Re: [PATCH 7/7] arm64: dts: qcom: ipq9574: add PCIe2 nodes
From: Krzysztof Kozlowski @ 2024-04-03 7:11 UTC (permalink / raw)
To: Alexandru Gagniuc, Bjorn Andersson, Konrad Dybcio, Rob Herring,
Krzysztof Kozlowski, Conor Dooley
Cc: ansuelsmth, robimarko, linux-arm-msm, devicetree, linux-kernel
In-Reply-To: <20240402192555.1955204-7-mr.nuke.me@gmail.com>
On 02/04/2024 21:25, Alexandru Gagniuc wrote:
> On ipq9574, there are 4 PCIe controllers. Describe the pcie2 node, and
> its PHY in devicetree.
>
> Only pcie2 is described, because only hardware using that controller
> was available for testing.
>
> Signed-off-by: Alexandru Gagniuc <mr.nuke.me@gmail.com>
> ---
> arch/arm64/boot/dts/qcom/ipq9574.dtsi | 93 ++++++++++++++++++++++++++-
> 1 file changed, 92 insertions(+), 1 deletion(-)
>
> diff --git a/arch/arm64/boot/dts/qcom/ipq9574.dtsi b/arch/arm64/boot/dts/qcom/ipq9574.dtsi
> index 7f2e5cbf3bbb..626d6359d750 100644
> --- a/arch/arm64/boot/dts/qcom/ipq9574.dtsi
> +++ b/arch/arm64/boot/dts/qcom/ipq9574.dtsi
> @@ -300,7 +300,7 @@ gcc: clock-controller@1800000 {
> <0>,
> <0>,
> <0>,
> - <0>,
> + <&pcie2_qmp_phy>,
> <0>,
> <0>;
> #clock-cells = <1>;
> @@ -745,6 +745,97 @@ frame@b128000 {
> status = "disabled";
> };
> };
> +
> + pcie2_qmp_phy: phy@8c000 {
> + compatible = "qcom,ipq9574-qmp-gen3x2-pcie-phy";
> + reg = <0x0008c000 0x14f4>;
> +
> + clocks = <&gcc GCC_PCIE2_AUX_CLK>,
> + <&gcc GCC_PCIE2_AHB_CLK>,
> + <&gcc GCC_PCIE2_PIPE_CLK>,
> + <&gcc GCC_ANOC_PCIE2_2LANE_M_CLK>,
> + <&gcc GCC_SNOC_PCIE2_2LANE_S_CLK>;
> + clock-names = "aux",
> + "cfg_ahb",
> + "pipe",
> + "anoc",
> + "snoc";
> +
> + clock-output-names = "pcie_phy2_pipe_clk";
> + #clock-cells = <0>;
> + #phy-cells = <0>;
> +
> + resets = <&gcc GCC_PCIE2_PHY_BCR>,
> + <&gcc GCC_PCIE2PHY_PHY_BCR>;
> + reset-names = "phy",
> + "common";
> + status = "disabled";
> + };
> +
> + pcie2: pcie@20000000 {
> + compatible = "qcom,pcie-ipq9574";
> + reg = <0x20000000 0xf1d>,
> + <0x20000f20 0xa8>,
> + <0x20001000 0x1000>,
> + <0x00088000 0x4000>,
> + <0x20100000 0x1000>;
> + reg-names = "dbi", "elbi", "atu", "parf", "config";
Put ranges here.
Best regards,
Krzysztof
^ permalink raw reply
* Re: [PATCH 1/7] dt-bindings: clock: Add PCIe pipe related clocks for IPQ9574
From: Krzysztof Kozlowski @ 2024-04-03 7:10 UTC (permalink / raw)
To: Alexandru Gagniuc, Bjorn Andersson, Konrad Dybcio,
Michael Turquette, Stephen Boyd, Rob Herring, Krzysztof Kozlowski,
Conor Dooley
Cc: ansuelsmth, robimarko, linux-arm-msm, linux-clk, devicetree,
linux-kernel
In-Reply-To: <20240402192555.1955204-1-mr.nuke.me@gmail.com>
On 02/04/2024 21:25, Alexandru Gagniuc wrote:
> Add defines for the missing PCIe PIPE clocks.
>
> Signed-off-by: Alexandru Gagniuc <mr.nuke.me@gmail.com>
> ---
> include/dt-bindings/clock/qcom,ipq9574-gcc.h | 4 ++++
> 1 file changed, 4 insertions(+)
I did not get half of this patchset. Are you sure you are CC-ing everyone?
For this one:
Acked-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
Best regards,
Krzysztof
^ permalink raw reply
* Re: [PATCH v6 1/6] dt-bindings: interconnect: Add Qualcomm IPQ9574 support
From: Krzysztof Kozlowski @ 2024-04-03 7:09 UTC (permalink / raw)
To: Varadarajan Narayanan, andersson, konrad.dybcio, mturquette,
sboyd, robh, krzysztof.kozlowski+dt, conor+dt, djakov,
dmitry.baryshkov, quic_anusha, linux-arm-msm, linux-clk,
devicetree, linux-kernel, linux-pm
In-Reply-To: <20240402103406.3638821-2-quic_varada@quicinc.com>
On 02/04/2024 12:34, Varadarajan Narayanan wrote:
> +#define ICC_NSSNOC_NSSCC 10
> +#define ICC_NSSNOC_SNOC_0 11
> +#define ICC_NSSNOC_SNOC_1 12
> +#define ICC_NSSNOC_PCNOC_1 13
> +#define ICC_NSSNOC_QOSGEN_REF 14
> +#define ICC_NSSNOC_TIMEOUT_REF 15
> +#define ICC_NSSNOC_XO_DCD 16
> +#define ICC_NSSNOC_ATB 17
> +#define ICC_MEM_NOC_NSSNOC 18
> +#define ICC_NSSNOC_MEMNOC 19
> +#define ICC_NSSNOC_MEM_NOC_1 20
> +
> +#define ICC_NSSNOC_PPE 0
> +#define ICC_NSSNOC_PPE_CFG 1
> +#define ICC_NSSNOC_NSS_CSR 2
> +#define ICC_NSSNOC_IMEM_QSB 3
> +#define ICC_NSSNOC_IMEM_AHB 4
> +
> +#define MASTER(x) ((ICC_ ## x) * 2)
> +#define SLAVE(x) (MASTER(x) + 1)
You already received comment to make your bindings consistent with other
Qualcomm bindings. Now you repeat the same mistake.
No, that is neither consistent nor greppble.
Best regards,
Krzysztof
^ permalink raw reply
* Re: [PATCH] dt-bindings: mfd: syscon: Add missing simple syscon compatibles
From: Krzysztof Kozlowski @ 2024-04-03 7:07 UTC (permalink / raw)
To: Rob Herring, Krzysztof Kozlowski, Conor Dooley, Andrew Lunn,
Gregory Clement, Sebastian Hesselbarth, Lee Jones, Ray Jui,
Scott Branden, Broadcom internal kernel review list,
Matthias Brugger, AngeloGioacchino Del Regno
Cc: devicetree, linux-kernel, linux-arm-kernel, linux-mediatek
In-Reply-To: <20240402202413.757283-1-robh@kernel.org>
On 02/04/2024 22:24, Rob Herring wrote:
> Add various "simple" syscon compatibles which were undocumented or
> still documented with old text bindings.
>
> apm,xgene-csw, apm,xgene-efuse, apm,xgene-mcb, apm,xgene-rb,
> fsl,ls1088a-reset, marvell,armada-3700-cpu-misc,
> mediatek,mt2712-pctl-a-syscfg, mediatek,mt6397-pctl-pmic-syscfg, and
> mediatek,mt8173-pctl-a-syscfg were all undocumented, but are in use
> already. Remove the old text binding docs for the others.
>
> Signed-off-by: Rob Herring <robh@kernel.org>
> ---
Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
Best regards,
Krzysztof
^ permalink raw reply
* Re: [PATCH 5/5] arm64: dts: Add device tree source for the Au-Zone Maivin Starter Kit
From: Francesco Dolcini @ 2024-04-03 7:06 UTC (permalink / raw)
To: Laurent Pinchart, Shawn Guo
Cc: devicetree, imx, linux-arm-kernel, Trevor Zaharichuk, Greg Lytle,
Shawn Guo, Sascha Hauer, Pengutronix Kernel Team, Fabio Estevam,
Rob Herring, Krzysztof Kozlowski, Conor Dooley
In-Reply-To: <ZgyjE05p/1NZnzaK@dragon>
Hello Laurent,
On Wed, Apr 03, 2024 at 08:30:11AM +0800, Shawn Guo wrote:
> On Mon, Mar 25, 2024 at 10:32:45PM +0200, Laurent Pinchart wrote:
> > diff --git a/arch/arm64/boot/dts/freescale/imx8mp-maivin.dts b/arch/arm64/boot/dts/freescale/imx8mp-maivin.dts
> > new file mode 100644
> > index 000000000000..2d1c8e782465
> > --- /dev/null
> > +++ b/arch/arm64/boot/dts/freescale/imx8mp-maivin.dts
> > @@ -0,0 +1,236 @@
[...]
> > +/* Verdin I2C_2_DSI */
> > +&i2c2 {
> > + status = "okay";
> > +
> > + clock-frequency = <400000>;
> > + scl-gpios = <&gpio5 16 (GPIO_ACTIVE_HIGH | GPIO_OPEN_DRAIN)>;
> > + sda-gpios = <&gpio5 17 (GPIO_ACTIVE_HIGH | GPIO_OPEN_DRAIN)>;
>
> We usually end property list with 'status'.
This is now a written and explicit guideline, no longer tribal knowledge,
see https://docs.kernel.org/devicetree/bindings/dts-coding-style.html#order-of-properties-in-device-node
Francesco
^ permalink raw reply
* Re: [PATCH v18 2/9] usb: dwc3: core: Access XHCI address space temporarily to read port info
From: Johan Hovold @ 2024-04-03 7:06 UTC (permalink / raw)
To: Krishna Kurapati PSSNV
Cc: Thinh Nguyen, Krzysztof Kozlowski, Rob Herring, Bjorn Andersson,
Wesley Cheng, Konrad Dybcio, Greg Kroah-Hartman, Conor Dooley,
Felipe Balbi, devicetree@vger.kernel.org,
linux-arm-msm@vger.kernel.org, linux-usb@vger.kernel.org,
linux-kernel@vger.kernel.org, quic_ppratap@quicinc.com,
quic_jackp@quicinc.com, Johan Hovold
In-Reply-To: <39010f95-b08f-4a57-b3af-f34eb1069865@quicinc.com>
On Wed, Apr 03, 2024 at 10:54:25AM +0530, Krishna Kurapati PSSNV wrote:
> >> +static int dwc3_read_port_info(struct dwc3 *dwc)
> >> +{
> >> + void __iomem *base;
> >> + u8 major_revision;
> >> + u32 offset;
> >> + u32 val;
> >> +
> >> + /*
> >> + * Remap xHCI address space to access XHCI ext cap regs since it is
> >> + * needed to get information on number of ports present.
> >> + */
> >> + base = ioremap(dwc->xhci_resources[0].start,
> >> + resource_size(&dwc->xhci_resources[0]));
> >> + if (IS_ERR(base))
> >> + return PTR_ERR(base);
> >
> > Looks like you forgot to address some of the comments you said you'd
> > update previously if you submit a new version to the series.
> >
> > [*] https://lore.kernel.org/linux-usb/af73110d-e13e-4183-af11-aed869ac0a31@quicinc.com/
> >
>
> Apologies. I agree. I was too much focused on acpi removal and interrupt
> cleanup, I forgot the last comment you gave.
>
> Can I send in a separate patch for this ?
The series has not been merged yet so you can address both issues in a
v19. Perhaps wait a day or two in case Thinh has further comments.
Johan
^ permalink raw reply
* Re: [PATCH v4 4/4] remoteproc: stm32: Add support of an OP-TEE TA to load the firmware
From: Arnaud POULIQUEN @ 2024-04-03 7:04 UTC (permalink / raw)
To: Mathieu Poirier
Cc: Bjorn Andersson, Jens Wiklander, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, linux-stm32, linux-arm-kernel, linux-remoteproc,
linux-kernel, op-tee, devicetree
In-Reply-To: <ZgrW2avODv29vWNP@p14s>
On 4/1/24 17:46, Mathieu Poirier wrote:
> On Fri, Mar 29, 2024 at 11:57:43AM +0100, Arnaud POULIQUEN wrote:
>>
>>
>> On 3/27/24 18:14, Mathieu Poirier wrote:
>>> On Tue, Mar 26, 2024 at 08:31:33PM +0100, Arnaud POULIQUEN wrote:
>>>>
>>>>
>>>> On 3/25/24 17:51, Mathieu Poirier wrote:
>>>>> On Fri, Mar 08, 2024 at 03:47:08PM +0100, Arnaud Pouliquen wrote:
>>>>>> The new TEE remoteproc device is used to manage remote firmware in a
>>>>>> secure, trusted context. The 'st,stm32mp1-m4-tee' compatibility is
>>>>>> introduced to delegate the loading of the firmware to the trusted
>>>>>> execution context. In such cases, the firmware should be signed and
>>>>>> adhere to the image format defined by the TEE.
>>>>>>
>>>>>> Signed-off-by: Arnaud Pouliquen <arnaud.pouliquen@foss.st.com>
>>>>>> ---
>>>>>> Updates from V3:
>>>>>> - remove support of the attach use case. Will be addressed in a separate
>>>>>> thread,
>>>>>> - add st_rproc_tee_ops::parse_fw ops,
>>>>>> - inverse call of devm_rproc_alloc()and tee_rproc_register() to manage cross
>>>>>> reference between the rproc struct and the tee_rproc struct in tee_rproc.c.
>>>>>> ---
>>>>>> drivers/remoteproc/stm32_rproc.c | 60 +++++++++++++++++++++++++++++---
>>>>>> 1 file changed, 56 insertions(+), 4 deletions(-)
>>>>>>
>>>>>> diff --git a/drivers/remoteproc/stm32_rproc.c b/drivers/remoteproc/stm32_rproc.c
>>>>>> index 8cd838df4e92..13df33c78aa2 100644
>>>>>> --- a/drivers/remoteproc/stm32_rproc.c
>>>>>> +++ b/drivers/remoteproc/stm32_rproc.c
>>>>>> @@ -20,6 +20,7 @@
>>>>>> #include <linux/remoteproc.h>
>>>>>> #include <linux/reset.h>
>>>>>> #include <linux/slab.h>
>>>>>> +#include <linux/tee_remoteproc.h>
>>>>>> #include <linux/workqueue.h>
>>>>>>
>>>>>> #include "remoteproc_internal.h"
>>>>>> @@ -49,6 +50,9 @@
>>>>>> #define M4_STATE_STANDBY 4
>>>>>> #define M4_STATE_CRASH 5
>>>>>>
>>>>>> +/* Remote processor unique identifier aligned with the Trusted Execution Environment definitions */
>>>>>
>>>>> Why is this the case? At least from the kernel side it is possible to call
>>>>> tee_rproc_register() with any kind of value, why is there a need to be any
>>>>> kind of alignment with the TEE?
>>>>
>>>>
>>>> The use of the proc_id is to identify a processor in case of multi co-processors.
>>>>
>>>
>>> That is well understood.
>>>
>>>> For instance we can have a system with A DSP and a modem. We would use the same
>>>> TEE service, but
>>>
>>> That too.
>>>
>>>> the TEE driver will probably be different, same for the signature key.
>>>
>>> What TEE driver are we talking about here?
>>
>> In OP-TEE remoteproc frameork is divided in 2 or 3 layers:
>>
>> - the remoteproc Trusted Application (TA) [1] which is platform agnostic
>> - The remoteproc Pseudo Trusted Application (PTA) [2] which is platform
>> dependent and can rely on the proc ID to retrieve the context.
>> - the remoteproc driver (optional for some platforms) [3], which is in charge
>> of DT parsing and platform configuration.
>>
>
> That part makes sense.
>
>> Here TEE driver can be interpreted by remote PTA and/or platform driver.
>>
>
> I have to guess PTA means "Platform Trusted Application" but I have no
> guarantee, adding to the level of (already high) confusion brought on by this
> patchset.
As mentioned above, PTA is Pseudo Trusted Application. It is an interface
exposed by OP-TEE core to the OP-TEE application.
In this case PTA is used to implement the platform part.
If you need more details about the remoteproc framework in OP-TEE, there is a
link in the to a presentation in the cover letter.
>
>> [1]
>> https://elixir.bootlin.com/op-tee/latest/source/ta/remoteproc/src/remoteproc_core.c
>> [2]
>> https://elixir.bootlin.com/op-tee/latest/source/core/pta/stm32mp/remoteproc_pta.c
>> [3]
>> https://elixir.bootlin.com/op-tee/latest/source/core/drivers/remoteproc/stm32_remoteproc.c
>>
>>>
>>>> In such case the proc ID allows to identify the the processor you want to address.
>>>>
>>>
>>> That too is well understood, but there is no alignment needed with the TEE, i.e
>>> the TEE application is not expecting a value of '0'. We could set
>>> STM32_MP1_M4_PROC_ID to 0xDEADBEEF and things would work. This driver won't go
>>> anywhere for as long as it is not the case.
>>
>>
>> Here I suppose that you do not challenge the rproc_ID use in general, but for
>> the stm32mp1 platform as we have only one remote processor. I'm right?
>
> That is correct - I understand the need for an rproc_ID. The problem is with
> the comment that states that '0' is aligned with the TEE definitions, which in
> my head means hard coded value and a big red flag. What it should say is
> "aligned with the TEE device tree definition".
>
>>
>> In OP-TEE the check is done here:
>> https://elixir.bootlin.com/op-tee/latest/source/core/drivers/remoteproc/stm32_remoteproc.c#L64
>>
>> If driver does not register the proc ID an error is returned indicating that the
>> feature is not supported.
>>
>> In case of stm32mp1 yes we could consider it as useless as we have only one
>> remote proc.
>>
>> Nevertheless I can not guaranty that a customer will not add an external
>> companion processor that uses OP-TEE to authenticate the associated firmware. As
>> the trusted Application is the unique entry point. he will need the proc_id to
>> identify the target at PTA level.
>>
>> So from my point of view having a proc ID on stm32MP1 (and on stm32mp2 that will
>> reuse same driver) aligned between Linux and OP-TEE is useful.
>
> I agree, for as long as it is not hard coded. The way remote processors are
> discovered in the DT is perfectly acceptable, i.e the first remote processor is
> for application X, the second for application Y...
>
>>
>> That said using a TEE_REMOTEPROC_DEFAULT_ID is something that could be
>> more generic (for linux and OP-TEE). This ID could be reuse in the stm32mp
>> driver and platform drivers with an unique internal remote processor.
>>
>
> I can't find the definition of TEE_REMOTEPROC_DEFAULT_ID anywhere, something
> that doesn't help the confusion I referred to above.
The TEE_REMOTEPROC_DEFAULT_ID does not yet exist; it is a proposal.
Nevertheless I also had in mind the addition of a "proc ID" property in the DT.
I will proceed in this way
I think I now have all the information needed to prepare a new revision.
Thanks for the time passed on this series and your advises
Regards,
Arnaud
>
>> It that solution would be ok for you?
>>
>> Regards,
>> Arnaud
>>
>>
>>>
>>>>
>>>>>
>>>>>> +#define STM32_MP1_M4_PROC_ID 0
>>>>>> +
>>>>>> struct stm32_syscon {
>>>>>> struct regmap *map;
>>>>>> u32 reg;
>>>>>> @@ -257,6 +261,19 @@ static int stm32_rproc_release(struct rproc *rproc)
>>>>>> return 0;
>>>>>> }
>>>>>>
>>>>>> +static int stm32_rproc_tee_stop(struct rproc *rproc)
>>>>>> +{
>>>>>> + int err;
>>>>>> +
>>>>>> + stm32_rproc_request_shutdown(rproc);
>>>>>> +
>>>>>> + err = tee_rproc_stop(rproc);
>>>>>> + if (err)
>>>>>> + return err;
>>>>>> +
>>>>>> + return stm32_rproc_release(rproc);
>>>>>> +}
>>>>>> +
>>>>>> static int stm32_rproc_prepare(struct rproc *rproc)
>>>>>> {
>>>>>> struct device *dev = rproc->dev.parent;
>>>>>> @@ -693,8 +710,19 @@ static const struct rproc_ops st_rproc_ops = {
>>>>>> .get_boot_addr = rproc_elf_get_boot_addr,
>>>>>> };
>>>>>>
>>>>>> +static const struct rproc_ops st_rproc_tee_ops = {
>>>>>> + .prepare = stm32_rproc_prepare,
>>>>>> + .start = tee_rproc_start,
>>>>>> + .stop = stm32_rproc_tee_stop,
>>>>>> + .kick = stm32_rproc_kick,
>>>>>> + .load = tee_rproc_load_fw,
>>>>>> + .parse_fw = tee_rproc_parse_fw,
>>>>>> + .find_loaded_rsc_table = tee_rproc_find_loaded_rsc_table,
>>>>>> +};
>>>>>> +
>>>>>> static const struct of_device_id stm32_rproc_match[] = {
>>>>>> - { .compatible = "st,stm32mp1-m4" },
>>>>>> + {.compatible = "st,stm32mp1-m4",},
>>>>>> + {.compatible = "st,stm32mp1-m4-tee",},
>>>>>> {},
>>>>>> };
>>>>>> MODULE_DEVICE_TABLE(of, stm32_rproc_match);
>>>>>> @@ -853,6 +881,7 @@ static int stm32_rproc_probe(struct platform_device *pdev)
>>>>>> struct device *dev = &pdev->dev;
>>>>>> struct stm32_rproc *ddata;
>>>>>> struct device_node *np = dev->of_node;
>>>>>> + struct tee_rproc *trproc = NULL;
>>>>>> struct rproc *rproc;
>>>>>> unsigned int state;
>>>>>> int ret;
>>>>>> @@ -861,9 +890,26 @@ static int stm32_rproc_probe(struct platform_device *pdev)
>>>>>> if (ret)
>>>>>> return ret;
>>>>>>
>>>>>> - rproc = devm_rproc_alloc(dev, np->name, &st_rproc_ops, NULL, sizeof(*ddata));
>>>>>> - if (!rproc)
>>>>>> - return -ENOMEM;
>>>>>> + if (of_device_is_compatible(np, "st,stm32mp1-m4-tee")) {
>>>>>> + /*
>>>>>> + * Delegate the firmware management to the secure context.
>>>>>> + * The firmware loaded has to be signed.
>>>>>> + */
>>>>>> + rproc = devm_rproc_alloc(dev, np->name, &st_rproc_tee_ops, NULL, sizeof(*ddata));
>>>>>> + if (!rproc)
>>>>>> + return -ENOMEM;
>>>>>> +
>>>>>> + trproc = tee_rproc_register(dev, rproc, STM32_MP1_M4_PROC_ID);
>>>>>> + if (IS_ERR(trproc)) {
>>>>>> + dev_err_probe(dev, PTR_ERR(trproc),
>>>>>> + "signed firmware not supported by TEE\n");
>>>>>> + return PTR_ERR(trproc);
>>>>>> + }
>>>>>> + } else {
>>>>>> + rproc = devm_rproc_alloc(dev, np->name, &st_rproc_ops, NULL, sizeof(*ddata));
>>>>>> + if (!rproc)
>>>>>> + return -ENOMEM;
>>>>>> + }
>>>>>>
>>>>>> ddata = rproc->priv;
>>>>>>
>>>>>> @@ -915,6 +961,9 @@ static int stm32_rproc_probe(struct platform_device *pdev)
>>>>>> dev_pm_clear_wake_irq(dev);
>>>>>> device_init_wakeup(dev, false);
>>>>>> }
>>>>>> + if (trproc)
>>>>>
>>>>> if (rproc->tee_interface)
>>>>>
>>>>>
>>>>> I am done reviewing this set.
>>>>
>>>> Thank for your review!
>>>> Arnaud
>>>>
>>>>>
>>>>> Thanks,
>>>>> Mathieu
>>>>>
>>>>>> + tee_rproc_unregister(trproc);
>>>>>> +
>>>>>> return ret;
>>>>>> }
>>>>>>
>>>>>> @@ -935,6 +984,9 @@ static void stm32_rproc_remove(struct platform_device *pdev)
>>>>>> dev_pm_clear_wake_irq(dev);
>>>>>> device_init_wakeup(dev, false);
>>>>>> }
>>>>>> + if (rproc->tee_interface)
>>>>>> + tee_rproc_unregister(rproc->tee_interface);
>>>>>> +
>>>>>> }
>>>>>>
>>>>>> static int stm32_rproc_suspend(struct device *dev)
>>>>>> --
>>>>>> 2.25.1
>>>>>>
^ permalink raw reply
* Re: [PATCH v2 1/4] dt-bindings: input: Add Himax HX83102J touchscreen
From: Allen Lin @ 2024-04-03 6:53 UTC (permalink / raw)
To: Conor Dooley
Cc: dmitry.torokhov, robh, krzysztof.kozlowski+dt, jikos,
benjamin.tissoires, linux-input, devicetree, linux-kernel
In-Reply-To: <20240402-doable-routine-8e8cb4f07ffb@spud>
Conor Dooley <conor@kernel.org> 於 2024年4月3日 週三 上午2:09寫道：
>
> On Tue, Apr 02, 2024 at 06:49:27PM +0800, Allen_Lin wrote:
> > Add the HX83102j touchscreen device tree bindings documents.
> > HX83102j is a Himax TDDI touchscreen controller.
> > It's power sequence should be bound with a lcm driver, thus it
> > needs to be a panel follower. Others are the same as normal SPI
> > touchscreen controller.
> >
> > Signed-off-by: Allen_Lin <allencl_lin@hotmail.com>
> > ---
> > .../input/touchscreen/himax,hx83102j.yaml | 100 ++++++++++++++++++
> > MAINTAINERS | 6 ++
> > 2 files changed, 106 insertions(+)
> > create mode 100644 Documentation/devicetree/bindings/input/touchscreen/himax,hx83102j.yaml
> >
> > diff --git a/Documentation/devicetree/bindings/input/touchscreen/himax,hx83102j.yaml b/Documentation/devicetree/bindings/input/touchscreen/himax,hx83102j.yaml
> > new file mode 100644
> > index 000000000000..fe79129f704a
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/input/touchscreen/himax,hx83102j.yaml
> > @@ -0,0 +1,100 @@
> > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> > +%YAML 1.2
> > +---
> > +$id: http://devicetree.org/schemas/input/touchscreen/himax,hx83102j.yaml#
> > +$schema: http://devicetree.org/meta-schemas/core.yaml#
> > +
> > +title: Himax hx83102j touchscreen
> > +
> > +maintainers:
> > + - Allen Lin <allencl_lin@hotmail.com>
> > +
> > +description:
> > + This Himax hx83102j touchscreen uses the spi protocol.
> > +
> > +allOf:
> > + - $ref: /schemas/input/touchscreen/touchscreen.yaml#
> > + - $ref: /schemas/spi/spi-peripheral-props.yaml#
> > +
> > +properties:
> > + compatible:
> > + const: himax,hx83102j
> > +
> > + reg:
> > + maxItems: 1
> > +
> > + interrupts:
> > + maxItems: 1
> > +
> > + reset-gpios:
> > + maxItems: 1
> > +
> > + vccd-supply:
> > + description: A phandle for the regualtor supplying IO power.
>
> nit: regulator
>
> > +
> > + vsn-supply:
> > + description: Negative supply regulator.
> > +
> > + vsp-supply:
> > + description: Positive supply regulator.
>
> Cool, thanks for adding these.
>
> > +
> > + ddreset-gpios:
> > + description: A phandle of gpio for display reset controlled by the LCD driver.
> > + This is the master reset, if this reset is triggered, the TP reset will
> > + also be triggered.
> > +
> > + spi-cpha: true
> > +
> > + spi-cpol: true
> > +
> > + spi-max-frequency: true
> > +
> > + panel: true
> > +
> > + himax,firmware-name:
>
> firmware-name is a standard property, you don't need to vendor prefix it.
>
> > + $ref: /schemas/types.yaml#/definitions/string
> > + description:
> > + Specify the file name for firmware loading.
> > +
> > + himax,pid:
> > + $ref: /schemas/types.yaml#/definitions/uint32
> > + description:
> > + PID for HID device, used to validate firmware.
>
> Why do you need this _and_ firmware-name? You should be able to trust
> the firmware that the dt has told you to use, no?
>
> Cheers,
> Conor.
OK, I should trust the dt document and fix these issues in next patch
1. fix "regulator" spelling
2. himax,firmware-name ->firmware-name
3. remove himax,pid
Thanks
Allen
>
> > +
> > +required:
> > + - compatible
> > + - reg
> > + - interrupts
> > + - reset-gpios
> > + - panel
> > + - vccd-supply
> > + - vsn-supply
> > + - vsp-supply
> > + - ddreset-gpios
> > +
> > +additionalProperties: false
> > +
> > +examples:
> > + - |
> > + #include <dt-bindings/gpio/gpio.h>
> > + #include <dt-bindings/interrupt-controller/irq.h>
> > + spi {
> > + #address-cells = <1>;
> > + #size-cells = <0>;
> > + ap_ts: touchscreen@0 {
> > + compatible = "himax,hx83102j";
> > + reg = <0>;
> > + pinctrl-names = "default";
> > + pinctrl-0 = <&touch_int0 &touch_reset>;
> > + reset-gpios = <&gpio1 8 GPIO_ACTIVE_LOW>;
> > + spi-cpha;
> > + spi-cpol;
> > + interrupt-parent = <&gpio1>;
> > + interrupts = <7 IRQ_TYPE_LEVEL_LOW>;
> > + panel = <&panel>;
> > + vccd-supply = <®ulator>;
> > + vsn-supply = <®ulator>;
> > + vsp-supply = <®ulator>;
> > + ddreset-gpios = <&gpio1>;
> > + };
> > + };
> > diff --git a/MAINTAINERS b/MAINTAINERS
> > index 43b39956694a..aa51c60fd66d 100644
> > --- a/MAINTAINERS
> > +++ b/MAINTAINERS
> > @@ -9669,6 +9669,12 @@ L: linux-kernel@vger.kernel.org
> > S: Maintained
> > F: drivers/misc/hisi_hikey_usb.c
> >
> > +HIMAX HID HX83102J TOUCHSCREEN
> > +M: Allen Lin <allencl_lin@hotmail.com>
> > +L: linux-input@vger.kernel.org
> > +S: Maintained
> > +F: Documentation/devicetree/bindings/input/touchscreen/himax,hx83102j.yaml
> > +
> > HIMAX HX83112B TOUCHSCREEN SUPPORT
> > M: Job Noorman <job@noorman.info>
> > L: linux-input@vger.kernel.org
> > --
> > 2.34.1
> >
^ permalink raw reply
* Re: [PATCH v1 1/5] dt-bindings: pwm: Add Loongson PWM controller
From: Krzysztof Kozlowski @ 2024-04-03 7:00 UTC (permalink / raw)
To: Binbin Zhou, Rob Herring
Cc: Binbin Zhou, Huacai Chen, Uwe Kleine-König,
Krzysztof Kozlowski, Conor Dooley, Huacai Chen, loongson-kernel,
linux-pwm, devicetree, Xuerui Wang, loongarch
In-Reply-To: <CAMpQs4K_VSqdm7x=cSyMTBYQyOm=th0YrYKdZ74dp35hyRBXgQ@mail.gmail.com>
On 03/04/2024 04:37, Binbin Zhou wrote:
> Hi Rob:
>
> Thanks for your reply.
>
> On Tue, Apr 2, 2024 at 11:40 PM Rob Herring <robh@kernel.org> wrote:
>>
>> On Tue, Apr 02, 2024 at 03:58:38PM +0800, Binbin Zhou wrote:
>>> Add Loongson PWM controller binding with DT schema format using
>>> json-schema.
>>>
>>> Signed-off-by: Binbin Zhou <zhoubinbin@loongson.cn>
>>> ---
>>> .../devicetree/bindings/pwm/pwm-loongson.yaml | 64 +++++++++++++++++++
>>
>> Filename should match compatible.
>
> Emm... How about renaming it as loongson, pwm.yaml?
Use the fallback, so loongson,ls7a-pwm.yaml
Best regards,
Krzysztof
^ permalink raw reply
* Re: [Patch v2 1/2] dt-bindings: make sid and broadcast reg optional
From: Krzysztof Kozlowski @ 2024-04-03 6:56 UTC (permalink / raw)
To: Jon Hunter, Sumit Gupta, robh, conor+dt, maz, mark.rutland,
treding
Cc: devicetree, linux-kernel, linux-tegra, amhetre, bbasu
In-Reply-To: <025ed42a-c6f2-48e6-a8d1-b6de79d6957b@nvidia.com>
On 02/04/2024 21:15, Jon Hunter wrote:
>
>
> On 02/04/2024 14:26, Sumit Gupta wrote:
>> MC SID and Broadbast channel register access is restricted for Guest VM.
>> Make both the regions as optional for SoC's from Tegra186 onwards.
>> Tegra MC driver will skip access to the restricted registers from Guest
>> if the respective regions are not present in the memory-controller node
>> of Guest DT.
>>
>> Signed-off-by: Sumit Gupta <sumitg@nvidia.com>
>> ---
>> .../memory-controllers/nvidia,tegra186-mc.yaml | 18 +++++++++---------
>> 1 file changed, 9 insertions(+), 9 deletions(-)
>>
>> diff --git a/Documentation/devicetree/bindings/memory-controllers/nvidia,tegra186-mc.yaml b/Documentation/devicetree/bindings/memory-controllers/nvidia,tegra186-mc.yaml
>> index 935d63d181d9..c52c259f7ec5 100644
>> --- a/Documentation/devicetree/bindings/memory-controllers/nvidia,tegra186-mc.yaml
>> +++ b/Documentation/devicetree/bindings/memory-controllers/nvidia,tegra186-mc.yaml
>> @@ -146,17 +146,17 @@ allOf:
>> then:
>> properties:
>> reg:
>> - maxItems: 6
>> + maxItems: 4
>
> minItems?
>
If the intention was to make it variable, then yes, missing minItems.
But more important: why patch was sent without any testing?
Best regards,
Krzysztof
^ permalink raw reply
* [PATCH v4 9/9] mailbox: mediatek: Add secure CMDQ driver support for CMDQ driver
From: Shawn Sung @ 2024-04-03 6:56 UTC (permalink / raw)
To: CK Hu, Jassi Brar, AngeloGioacchino Del Regno
Cc: Rob Herring, Krzysztof Kozlowski, Conor Dooley, Matthias Brugger,
Hsiao Chien Sung, Jason-JH . Lin, Houlong Wei, linux-kernel,
devicetree, linux-arm-kernel, linux-mediatek
In-Reply-To: <20240403065603.21920-1-shawn.sung@mediatek.com>
From: "Jason-JH.Lin" <jason-jh.lin@mediatek.com>
CMDQ driver will probe a secure CMDQ driver when has_sec flag
in platform data is true and its device node in dts has defined a
event id of CMDQ_SYNC_TOKEN_SEC_EOF.
Secure CMDQ driver support on mt8188 and mt8195 currently.
So add a has_secure flag to their driver data to probe it.
Signed-off-by: Jason-JH.Lin <jason-jh.lin@mediatek.com>
Signed-off-by: Hsiao Chien Sung <shawn.sung@mediatek.com>
---
drivers/mailbox/mtk-cmdq-mailbox.c | 38 ++++++++++++++++++++++++++++++
1 file changed, 38 insertions(+)
diff --git a/drivers/mailbox/mtk-cmdq-mailbox.c b/drivers/mailbox/mtk-cmdq-mailbox.c
index e04302ca6ec03..a8a0619baaa5c 100644
--- a/drivers/mailbox/mtk-cmdq-mailbox.c
+++ b/drivers/mailbox/mtk-cmdq-mailbox.c
@@ -15,6 +15,7 @@
#include <linux/pm_runtime.h>
#include <linux/mailbox_controller.h>
#include <linux/mailbox/mtk-cmdq-mailbox.h>
+#include <linux/mailbox/mtk-cmdq-sec-mailbox.h>
#include <linux/of.h>
#define CMDQ_MBOX_AUTOSUSPEND_DELAY_MS 100
@@ -60,6 +61,9 @@ struct gce_plat {
u8 shift;
bool control_by_sw;
bool sw_ddr_en;
+ bool has_secure;
+ u32 secure_thread_nr;
+ u32 secure_thread_min;
u32 gce_num;
};
@@ -569,6 +573,7 @@ static int cmdq_probe(struct platform_device *pdev)
int alias_id = 0;
static const char * const clk_name = "gce";
static const char * const clk_names[] = { "gce0", "gce1" };
+ u32 hwid = 0;
cmdq = devm_kzalloc(dev, sizeof(*cmdq), GFP_KERNEL);
if (!cmdq)
@@ -594,6 +599,8 @@ static int cmdq_probe(struct platform_device *pdev)
dev, cmdq->base, cmdq->irq);
if (cmdq->pdata->gce_num > 1) {
+ hwid = of_alias_get_id(dev->of_node, clk_name);
+
for_each_child_of_node(phandle->parent, node) {
alias_id = of_alias_get_id(node, clk_name);
if (alias_id >= 0 && alias_id < cmdq->pdata->gce_num) {
@@ -676,6 +683,31 @@ static int cmdq_probe(struct platform_device *pdev)
pm_runtime_set_autosuspend_delay(dev, CMDQ_MBOX_AUTOSUSPEND_DELAY_MS);
pm_runtime_use_autosuspend(dev);
+ if (cmdq->pdata->has_secure) {
+ struct platform_device *mtk_cmdq_sec;
+ static struct gce_sec_plat sec_plat = {0};
+
+ if (of_property_read_u32_index(dev->of_node, "mediatek,gce-events", 0,
+ &sec_plat.cmdq_event) == 0) {
+ sec_plat.gce_dev = dev;
+ sec_plat.hwid = hwid;
+ sec_plat.gce_num = cmdq->pdata->gce_num;
+ sec_plat.clocks = cmdq->clocks;
+ sec_plat.thread_nr = cmdq->pdata->thread_nr;
+ sec_plat.secure_thread_nr = cmdq->pdata->secure_thread_nr;
+ sec_plat.secure_thread_min = cmdq->pdata->secure_thread_min;
+
+ mtk_cmdq_sec = platform_device_register_data(dev, "mtk-cmdq-sec",
+ PLATFORM_DEVID_AUTO,
+ &sec_plat,
+ sizeof(sec_plat));
+ if (IS_ERR(mtk_cmdq_sec)) {
+ dev_err(dev, "failed to register platform_device mtk-cmdq-sec\n");
+ return PTR_ERR(mtk_cmdq_sec);
+ }
+ }
+ }
+
return 0;
}
@@ -719,6 +751,9 @@ static const struct gce_plat gce_plat_mt8188 = {
.thread_nr = 32,
.shift = 3,
.control_by_sw = true,
+ .has_secure = true,
+ .secure_thread_nr = 2,
+ .secure_thread_min = 8,
.gce_num = 2
};
@@ -733,6 +768,9 @@ static const struct gce_plat gce_plat_mt8195 = {
.thread_nr = 24,
.shift = 3,
.control_by_sw = true,
+ .has_secure = true,
+ .secure_thread_nr = 2,
+ .secure_thread_min = 8,
.gce_num = 2
};
--
2.18.0
^ permalink raw reply related
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox