* Re: [PATCH v3 23/25] drivers: media: i2c: imx258: Add support for powerdown gpio
From: Luis Garcia @ 2024-04-04 23:18 UTC (permalink / raw)
To: Dave Stevenson
Cc: Ondřej Jirman, Sakari Ailus, linux-media, jacopo.mondi,
mchehab, robh, krzysztof.kozlowski+dt, conor+dt, shawnguo,
s.hauer, kernel, festevam, devicetree, imx, linux-arm-kernel,
linux-kernel, pavel, phone-devel
In-Reply-To: <CAPY8ntAcB3wyLj1wNE5YBx0_UGRiXEv6057XfEBfjk8NOLC9yQ@mail.gmail.com>
On 4/4/24 08:12, Dave Stevenson wrote:
> Hi Luigi
>
> On Wed, 3 Apr 2024 at 20:34, Luigi311 <git@luigi311.com> wrote:
>>
>> On 4/3/24 10:57, Ondřej Jirman wrote:
>>> Hi Sakari and Luis,
>>>
>>> On Wed, Apr 03, 2024 at 04:25:41PM GMT, Sakari Ailus wrote:
>>>> Hi Luis, Ondrej,
>>>>
>>>> On Wed, Apr 03, 2024 at 09:03:52AM -0600, git@luigi311.com wrote:
>>>>> From: Luis Garcia <git@luigi311.com>
>>>>>
>>>>> On some boards powerdown signal needs to be deasserted for this
>>>>> sensor to be enabled.
>>>>>
>>>>> Signed-off-by: Ondrej Jirman <megi@xff.cz>
>>>>> Signed-off-by: Luis Garcia <git@luigi311.com>
>>>>> ---
>>>>> drivers/media/i2c/imx258.c | 13 +++++++++++++
>>>>> 1 file changed, 13 insertions(+)
>>>>>
>>>>> diff --git a/drivers/media/i2c/imx258.c b/drivers/media/i2c/imx258.c
>>>>> index 30352c33f63c..163f04f6f954 100644
>>>>> --- a/drivers/media/i2c/imx258.c
>>>>> +++ b/drivers/media/i2c/imx258.c
>>>>> @@ -679,6 +679,8 @@ struct imx258 {
>>>>> unsigned int lane_mode_idx;
>>>>> unsigned int csi2_flags;
>>>>>
>>>>> + struct gpio_desc *powerdown_gpio;
>>>>> +
>>>>> /*
>>>>> * Mutex for serialized access:
>>>>> * Protect sensor module set pad format and start/stop streaming safely.
>>>>> @@ -1213,6 +1215,8 @@ static int imx258_power_on(struct device *dev)
>>>>> struct imx258 *imx258 = to_imx258(sd);
>>>>> int ret;
>>>>>
>>>>> + gpiod_set_value_cansleep(imx258->powerdown_gpio, 0);
>>>>
>>>> What does the spec say? Should this really happen before switching on the
>>>> supplies below?
>>>
>>> There's no powerdown input in the IMX258 manual. The manual only mentions
>>> that XCLR (reset) should be held low during power on.
>>>
>>> https://megous.com/dl/tmp/15b0992a720ab82d.png
>>>
>>> https://megous.com/dl/tmp/f2cc991046d97641.png
>>>
>>> This sensor doesn’t have a built-in “Power ON Reset” function. The XCLR pin
>>> is set to “LOW” and the power supplies are brought up. Then the XCLR pin
>>> should be set to “High” after INCK supplied.
>>>
>>> So this input is some feature on camera module itself outside of the
>>> IMX258 chip, which I think is used to gate power to the module. Eg. on Pinephone
>>> Pro, there are two modules with shared power rails, so enabling supply to
>>> one module enables it to the other one, too. So this input becomes the only way
>>> to really enable/disable power to the chip when both are used at once at some
>>> point, because regulator_bulk_enable/disable becomes ineffective at that point.
>>>
>>> Luis, maybe you saw some other datasheet that mentions this input? IMO,
>>> it just gates the power rails via some mosfets on the module itself, since
>>> there's not power down input to the chip itself.
>>>
>>> kind regards,
>>> o.
>>>
>>
>> Ondrej, I did not see anything else in the datasheet since I'm pretty sure
>> I'm looking at the same datasheet as it was supplied to me by Pine64. I'm
>> not sure what datasheet Dave has access to since he got his for a
>> completely different module than what we are testing with though.
>
> I only have a leaked datasheet (isn't the internet wonderful!) [1]
> XCLR is documented in that, as Ondrej has said.
>
> If this powerdown GPIO is meant to be driving XCLR, then it is in the
> wrong order against the supplies.
>
> This does make me confused over the difference between this powerdown
> GPIO and the reset GPIO that you implement in 24/25.
>
> Following the PinePhone Pro DT [3] and schematics [4]
> reset-gpios = <&gpio1 RK_PA0 GPIO_ACTIVE_LOW>;
> powerdown-gpios = <&gpio2 RK_PD4 GPIO_ACTIVE_HIGH>;
>
> Schematic page 11 upper right block
> GPIO1_A0/ISP0_SHUTTER_EN/ISP1_SHUTTER_EN/TCPD_VBUS_SINK_EN_d becomes
> Camera_RST_L. Page 18 feeds that through to the RESET on the camera
> connector.
> Page 11 left middle block GPIO2_D4/SDIO0_BKPWR_d becomes DVP_PDN1_H.
> Page 18 feeds that through to the PWDN on the camera connector.
>
> Seeing as we apparently have a lens driver kicking around as well,
> potentially one is reset to the VCM, and one to the sensor? DW9714
> does have an XSD shutdown pin.
> Only the module integrator is going to really know the answer,
> although potentially a little poking with gpioset and i2cdetect may
> tell you more.
>
> Dave
>
> [1] https://web.archive.org/web/20201027131326/www.hi.app/IMX258-datasheet.pdf
> [2] https://files.pine64.org/doc/PinePhonePro/PinephonePro-Schematic-V1.0-20211127.pdf
> [3] https://xff.cz/git/linux/tree/arch/arm64/boot/dts/rockchip/rk3399-pinephone-pro.dts?h=orange-pi-5.18#n868
> [4] https://files.pine64.org/doc/PinePhonePro/PinephonePro-Schematic-V1.0-20211127.pdf
>
>
Out of curiosity I dropped this and tested it on my PPP and it still loads
up the camera correctly so I am fine with dropping this and patch 22 that
adds in the dt binding
>>>>> +
>>>>> ret = regulator_bulk_enable(IMX258_NUM_SUPPLIES,
>>>>> imx258->supplies);
>>>>> if (ret) {
>>>>> @@ -1224,6 +1228,7 @@ static int imx258_power_on(struct device *dev)
>>>>> ret = clk_prepare_enable(imx258->clk);
>>>>> if (ret) {
>>>>> dev_err(dev, "failed to enable clock\n");
>>>>> + gpiod_set_value_cansleep(imx258->powerdown_gpio, 1);
>>>>> regulator_bulk_disable(IMX258_NUM_SUPPLIES, imx258->supplies);
>>>>> }
>>>>>
>>>>> @@ -1238,6 +1243,8 @@ static int imx258_power_off(struct device *dev)
>>>>> clk_disable_unprepare(imx258->clk);
>>>>> regulator_bulk_disable(IMX258_NUM_SUPPLIES, imx258->supplies);
>>>>>
>>>>> + gpiod_set_value_cansleep(imx258->powerdown_gpio, 1);
>>>>> +
>>>>> return 0;
>>>>> }
>>>>>
>>>>> @@ -1541,6 +1548,12 @@ static int imx258_probe(struct i2c_client *client)
>>>>> if (!imx258->variant_cfg)
>>>>> imx258->variant_cfg = &imx258_cfg;
>>>>>
>>>>> + /* request optional power down pin */
>>>>> + imx258->powerdown_gpio = devm_gpiod_get_optional(&client->dev, "powerdown",
>>>>> + GPIOD_OUT_HIGH);
>>>>> + if (IS_ERR(imx258->powerdown_gpio))
>>>>> + return PTR_ERR(imx258->powerdown_gpio);
>>>>> +
>>>>> /* Initialize subdev */
>>>>> v4l2_i2c_subdev_init(&imx258->sd, client, &imx258_subdev_ops);
>>>>>
>>>>
>>>> --
>>>> Regards,
>>>>
>>>> Sakari Ailus
>>
^ permalink raw reply
* Re: [PATCH 1/4] ARM: dts: aspeed: greatlakes: correct Mellanox multi-host property
From: Andrew Jeffery @ 2024-04-04 23:21 UTC (permalink / raw)
To: Krzysztof Kozlowski, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, Joel Stanley, devicetree, linux-arm-kernel,
linux-aspeed, linux-kernel
In-Reply-To: <c3902c6e-c38e-4604-b79e-2b5406274d8f@linaro.org>
On Thu, 2024-04-04 at 08:13 +0200, Krzysztof Kozlowski wrote:
> On 04/04/2024 03:41, Andrew Jeffery wrote:
> > On Wed, 2024-04-03 at 12:04 +0200, Krzysztof Kozlowski wrote:
> > > On Sat, 09 Dec 2023 11:44:09 +0100, Krzysztof Kozlowski wrote:
> > > > "mlx,multi-host" is using incorrect vendor prefix and is not documented.
> > > >
> > > >
> > >
> > > These wait for ~4 months and they were not picked up. Let me know if anyone
> > > else wants to take these.
> > >
> > > Applied, thanks!
> > >
> > > [1/4] ARM: dts: aspeed: greatlakes: correct Mellanox multi-host property
> > > https://git.kernel.org/krzk/linux-dt/c/7da85354c4fa35b862294dbbb450baeb405b5a92
> > > [2/4] ARM: dts: aspeed: minerva-cmc: correct Mellanox multi-host property
> > > https://git.kernel.org/krzk/linux-dt/c/e515719c17beb9625a90039f6c45fa36d58bdda2
> > > [3/4] ARM: dts: aspeed: yosemite4: correct Mellanox multi-host property
> > > https://git.kernel.org/krzk/linux-dt/c/af3deaf9bcb4571feb89a4050c7ad75de9aa8e1e
> > > [4/4] ARM: dts: aspeed: yosemitev2: correct Mellanox multi-host property
> > > https://git.kernel.org/krzk/linux-dt/c/cac1c1dda6130771e06ace030b1b0ed62096a912
> > >
> > > Best regards,
> >
> > Ah, my apologies. Joel's on leave and I'm accumulating patches in a
> > tree for him in the mean time. I've had some things going on
> > professionally (changed jobs) and personally, and these fell into a bit
> > of a hole.
> >
> > I'm okay for these patches to be integrated through your tree, given
> > you've already applied them. Feel free to add acks if your branch
> > allows:
> >
> > Acked-by: Andrew Jeffery <andrew@codeconstruct.com.au>
> >
> > I'm working to stay on top of things a bit more now than I have in the
> > recent past, so hopefully I won't miss patches again in the future.
>
> Stephen reported conflict, although trivial, but maybe better if you
> take them?
>
Yeah, happy to.
> I can rebase and resend.
Thanks.
Andrew
^ permalink raw reply
* Re: [PATCH 3/3] of: Use scope based of_node_put() cleanups
From: Saravana Kannan @ 2024-04-04 23:21 UTC (permalink / raw)
To: Rob Herring; +Cc: Jonathan Cameron, devicetree, linux-kernel
In-Reply-To: <20240404-dt-cleanup-free-v1-3-c60e6cba8da9@kernel.org>
On Thu, Apr 4, 2024 at 7:15 AM Rob Herring <robh@kernel.org> wrote:
>
> Use the relatively new scope based of_node_put() cleanup to simplify
> function exit handling. Doing so reduces the chances of forgetting an
> of_node_put() and simplifies error paths by avoiding the need for goto
> statements.
>
> Signed-off-by: Rob Herring <robh@kernel.org>
> ---
> drivers/of/address.c | 60 ++++++++++++++++-----------------------------------
> drivers/of/property.c | 22 ++++++-------------
> 2 files changed, 26 insertions(+), 56 deletions(-)
>
> diff --git a/drivers/of/address.c b/drivers/of/address.c
> index ae46a3605904..f7b2d535a6d1 100644
> --- a/drivers/of/address.c
> +++ b/drivers/of/address.c
> @@ -491,7 +491,6 @@ static u64 __of_translate_address(struct device_node *dev,
> const __be32 *in_addr, const char *rprop,
> struct device_node **host)
> {
> - struct device_node *parent = NULL;
> struct of_bus *bus, *pbus;
> __be32 addr[OF_MAX_ADDR_CELLS];
> int na, ns, pna, pns;
> @@ -504,7 +503,7 @@ static u64 __of_translate_address(struct device_node *dev,
>
> *host = NULL;
> /* Get parent & match bus type */
> - parent = get_parent(dev);
> + struct device_node *parent __free(device_node) = get_parent(dev);
Can we leave the variable definition where it was? We generally define
all the variables up top. So, defining the one variable in the middle
feels weird. I at least get when we do this inside for/if blocks. But
randomly in the middle feels weird.
Similar comments in other places. Since both kfree() and of_put() can
both handle NULL pointers, I'd be surprised if we HAVE to combine
these lines.
Not a very strong position, but I'd rather we didn't. So,
Reviewed-by: Saravana Kannan <saravanak@google.com>
-Saravana
> if (parent == NULL)
> goto bail;
> bus = of_match_bus(parent);
> @@ -573,7 +572,6 @@ static u64 __of_translate_address(struct device_node *dev,
> of_dump_addr("one level translation:", addr, na);
> }
> bail:
> - of_node_put(parent);
> of_node_put(dev);
>
> return result;
> @@ -654,19 +652,16 @@ EXPORT_SYMBOL(of_translate_dma_address);
> const __be32 *of_translate_dma_region(struct device_node *dev, const __be32 *prop,
> phys_addr_t *start, size_t *length)
> {
> - struct device_node *parent;
> + struct device_node *parent __free(device_node) = __of_get_dma_parent(dev);
> u64 address, size;
> int na, ns;
>
> - parent = __of_get_dma_parent(dev);
> if (!parent)
> return NULL;
>
> na = of_bus_n_addr_cells(parent);
> ns = of_bus_n_size_cells(parent);
>
> - of_node_put(parent);
> -
> address = of_translate_dma_address(dev, prop);
> if (address == OF_BAD_ADDR)
> return NULL;
> @@ -688,21 +683,19 @@ const __be32 *__of_get_address(struct device_node *dev, int index, int bar_no,
> {
> const __be32 *prop;
> unsigned int psize;
> - struct device_node *parent;
> + struct device_node *parent __free(device_node) = of_get_parent(dev);
> struct of_bus *bus;
> int onesize, i, na, ns;
>
> - /* Get parent & match bus type */
> - parent = of_get_parent(dev);
> if (parent == NULL)
> return NULL;
> +
> + /* match the parent's bus type */
> bus = of_match_bus(parent);
> - if (strcmp(bus->name, "pci") && (bar_no >= 0)) {
> - of_node_put(parent);
> + if (strcmp(bus->name, "pci") && (bar_no >= 0))
> return NULL;
> - }
> +
> bus->count_cells(dev, &na, &ns);
> - of_node_put(parent);
> if (!OF_CHECK_ADDR_COUNT(na))
> return NULL;
>
> @@ -888,14 +881,13 @@ static u64 of_translate_ioport(struct device_node *dev, const __be32 *in_addr,
> */
> int of_dma_get_range(struct device_node *np, const struct bus_dma_region **map)
> {
> - struct device_node *node = of_node_get(np);
> + struct device_node *node __free(device_node) = of_node_get(np);
> const __be32 *ranges = NULL;
> bool found_dma_ranges = false;
> struct of_range_parser parser;
> struct of_range range;
> struct bus_dma_region *r;
> int len, num_ranges = 0;
> - int ret = 0;
>
> while (node) {
> ranges = of_get_property(node, "dma-ranges", &len);
> @@ -905,10 +897,9 @@ int of_dma_get_range(struct device_node *np, const struct bus_dma_region **map)
> break;
>
> /* Once we find 'dma-ranges', then a missing one is an error */
> - if (found_dma_ranges && !ranges) {
> - ret = -ENODEV;
> - goto out;
> - }
> + if (found_dma_ranges && !ranges)
> + return -ENODEV;
> +
> found_dma_ranges = true;
>
> node = of_get_next_dma_parent(node);
> @@ -916,10 +907,8 @@ int of_dma_get_range(struct device_node *np, const struct bus_dma_region **map)
>
> if (!node || !ranges) {
> pr_debug("no dma-ranges found for node(%pOF)\n", np);
> - ret = -ENODEV;
> - goto out;
> + return -ENODEV;
> }
> -
> of_dma_range_parser_init(&parser, node);
> for_each_of_range(&parser, &range) {
> if (range.cpu_addr == OF_BAD_ADDR) {
> @@ -930,16 +919,12 @@ int of_dma_get_range(struct device_node *np, const struct bus_dma_region **map)
> num_ranges++;
> }
>
> - if (!num_ranges) {
> - ret = -EINVAL;
> - goto out;
> - }
> + if (!num_ranges)
> + return -EINVAL;
>
> r = kcalloc(num_ranges + 1, sizeof(*r), GFP_KERNEL);
> - if (!r) {
> - ret = -ENOMEM;
> - goto out;
> - }
> + if (!r)
> + return -ENOMEM;
>
> /*
> * Record all info in the generic DMA ranges array for struct device,
> @@ -957,9 +942,7 @@ int of_dma_get_range(struct device_node *np, const struct bus_dma_region **map)
> r->size = range.size;
> r++;
> }
> -out:
> - of_node_put(node);
> - return ret;
> + return 0;
> }
> #endif /* CONFIG_HAS_DMA */
>
> @@ -1016,11 +999,9 @@ phys_addr_t __init of_dma_get_max_cpu_address(struct device_node *np)
> */
> bool of_dma_is_coherent(struct device_node *np)
> {
> - struct device_node *node;
> + struct device_node *node __free(device_node) = of_node_get(np);
> bool is_coherent = dma_default_coherent;
>
> - node = of_node_get(np);
> -
> while (node) {
> if (of_property_read_bool(node, "dma-coherent")) {
> is_coherent = true;
> @@ -1032,7 +1013,6 @@ bool of_dma_is_coherent(struct device_node *np)
> }
> node = of_get_next_dma_parent(node);
> }
> - of_node_put(node);
> return is_coherent;
> }
> EXPORT_SYMBOL_GPL(of_dma_is_coherent);
> @@ -1049,19 +1029,17 @@ EXPORT_SYMBOL_GPL(of_dma_is_coherent);
> */
> static bool of_mmio_is_nonposted(struct device_node *np)
> {
> - struct device_node *parent;
> bool nonposted;
>
> if (!IS_ENABLED(CONFIG_ARCH_APPLE))
> return false;
>
> - parent = of_get_parent(np);
> + struct device_node *parent __free(device_node) = of_get_parent(np);
> if (!parent)
> return false;
>
> nonposted = of_property_read_bool(parent, "nonposted-mmio");
>
> - of_node_put(parent);
> return nonposted;
> }
>
> diff --git a/drivers/of/property.c b/drivers/of/property.c
> index a6358ee99b74..b73daf81c99d 100644
> --- a/drivers/of/property.c
> +++ b/drivers/of/property.c
> @@ -40,15 +40,12 @@
> */
> bool of_graph_is_present(const struct device_node *node)
> {
> - struct device_node *ports, *port;
> + struct device_node *ports __free(device_node) = of_get_child_by_name(node, "ports");
>
> - ports = of_get_child_by_name(node, "ports");
> if (ports)
> node = ports;
>
> - port = of_get_child_by_name(node, "port");
> - of_node_put(ports);
> - of_node_put(port);
> + struct device_node *port __free(device_node) = of_get_child_by_name(node, "port");
>
> return !!port;
> }
> @@ -610,9 +607,9 @@ EXPORT_SYMBOL(of_graph_parse_endpoint);
> */
> struct device_node *of_graph_get_port_by_id(struct device_node *parent, u32 id)
> {
> - struct device_node *node, *port;
> + struct device_node *port;
> + struct device_node *node __free(device_node) = of_get_child_by_name(parent, "ports");
>
> - node = of_get_child_by_name(parent, "ports");
> if (node)
> parent = node;
>
> @@ -626,8 +623,6 @@ struct device_node *of_graph_get_port_by_id(struct device_node *parent, u32 id)
> break;
> }
>
> - of_node_put(node);
> -
> return port;
> }
> EXPORT_SYMBOL(of_graph_get_port_by_id);
> @@ -655,14 +650,13 @@ struct device_node *of_graph_get_next_endpoint(const struct device_node *parent,
> * parent port node.
> */
> if (!prev) {
> - struct device_node *node;
> + struct device_node *node __free(device_node) =
> + of_get_child_by_name(parent, "ports");
>
> - node = of_get_child_by_name(parent, "ports");
> if (node)
> parent = node;
>
> port = of_get_child_by_name(parent, "port");
> - of_node_put(node);
>
> if (!port) {
> pr_debug("graph: no port node found in %pOF\n", parent);
> @@ -1052,15 +1046,13 @@ static int of_fwnode_graph_parse_endpoint(const struct fwnode_handle *fwnode,
> struct fwnode_endpoint *endpoint)
> {
> const struct device_node *node = to_of_node(fwnode);
> - struct device_node *port_node = of_get_parent(node);
> + struct device_node *port_node __free(device_node) = of_get_parent(node);
>
> endpoint->local_fwnode = fwnode;
>
> of_property_read_u32(port_node, "reg", &endpoint->port);
> of_property_read_u32(node, "reg", &endpoint->id);
>
> - of_node_put(port_node);
> -
> return 0;
> }
>
>
> --
> 2.43.0
>
^ permalink raw reply
* Re: [PATCH v19 2/9] usb: dwc3: core: Access XHCI address space temporarily to read port info
From: Bjorn Andersson @ 2024-04-05 1:25 UTC (permalink / raw)
To: Greg Kroah-Hartman
Cc: Krzysztof Kozlowski, Johan Hovold, Krishna Kurapati,
Krzysztof Kozlowski, Rob Herring, Bjorn Andersson, Wesley Cheng,
Konrad Dybcio, Conor Dooley, Thinh Nguyen, Felipe Balbi,
devicetree, linux-arm-msm, linux-usb, linux-kernel, quic_ppratap,
quic_jackp, Johan Hovold
In-Reply-To: <2024040455-sitting-dictator-170c@gregkh>
On Thu, Apr 04, 2024 at 02:58:29PM +0200, Greg Kroah-Hartman wrote:
> On Thu, Apr 04, 2024 at 10:07:27AM +0200, Krzysztof Kozlowski wrote:
> > On 04/04/2024 09:21, Johan Hovold wrote:
> > > On Thu, Apr 04, 2024 at 10:42:22AM +0530, Krishna Kurapati wrote:
> > >
> > >> +static int dwc3_get_num_ports(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 (!base)
> > >> + return PTR_ERR(base);
> > >
> > > This is obviously still broken. You need to update the return value as
> > > well.
> > >
> > > Fix in v20.
> >
> > If one patchset reaches 20 versions, I think it is time to stop and
> > really think from the beginning, why issues keep appearing and reviewers
> > are still not happy.
> >
> > Maybe you did not perform extensive internal review, which you are
> > encouraged to by your own internal policies, AFAIR. Before posting next
> > version, please really get some internal review first.
>
> Also get those internal reviewers to sign-off on the commits and have
> that show up when you post them next. That way they are also
> responsible for this patchset, it's not fair that they are making you do
> all the work here :)
>
I like this idea and I'm open to us changing our way of handling this.
But unless such internal review brings significant input to the
development I'd say a s-o-b would take the credit from the actual
author.
We've discussed a few times about carrying Reviewed-by et al from the
internal reviews, but as maintainer I dislike this because I'd have no
way to know if a r-b on vN means the patch was reviewed, or if it was
just "accidentally" carried from v(N-1).
But it might be worth this risk, is this something you think would be
appropriate?
Regards,
Bjorn
^ permalink raw reply
* [PATCH v8 0/4] firmware: arm_scmi: Add SCMI v3.2 pincontrol protocol basic support
From: Peng Fan (OSS) @ 2024-04-05 1:59 UTC (permalink / raw)
To: Sudeep Holla, Cristian Marussi, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, Linus Walleij, Dan Carpenter
Cc: Andy Shevchenko, linux-arm-kernel, linux-kernel, devicetree,
linux-gpio, Peng Fan, Oleksii Moisieiev
This patchset is a rework from Oleksii's RFC v5 patchset
https://lore.kernel.org/all/cover.1698353854.git.oleksii_moisieiev@epam.com/
This patchset introduces some changes based on RFC v5:
- introduce helper get_max_msg_size
- support compatible string
- iterate the id_table
- Support multiple configs in one command
- Added i.MX support
- Patch 5 firmware: arm_scmi: Add SCMI v3.2 pincontrol protocol basic support
is almost same as RFCv5 expect multiple configs support.
- Patch 4 the dt-bindings includes compatible string to support i.MX
- Rebased on 2023-12-15 linux-next/master
If any comments from RFC v5 are missed, I am sorry in advance.
This PINCTRL Protocol is following Version 3.2 SCMI Spec Beta release.
On ARM-based systems, a separate Cortex-M based System Control Processor
(SCP) provides control on pins, as well as with power, clocks, reset
controllers. So implement the driver to support such cases.
The i.MX95 Example as below:
Configuration:
The scmi-pinctrl driver can be configured using DT bindings.
For example:
/ {
sram0: sram@445b1000 {
compatible = "mmio-sram";
reg = <0x0 0x445b1000 0x0 0x400>;
#address-cells = <1>;
#size-cells = <1>;
ranges = <0x0 0x0 0x445b1000 0x400>;
scmi_buf0: scmi-sram-section@0 {
compatible = "arm,scmi-shmem";
reg = <0x0 0x80>;
};
scmi_buf1: scmi-sram-section@80 {
compatible = "arm,scmi-shmem";
reg = <0x80 0x80>;
};
};
firmware {
scmi {
compatible = "arm,scmi";
mboxes = <&mu2 5 0>, <&mu2 3 0>, <&mu2 3 1>;
shmem = <&scmi_buf0>, <&scmi_buf1>;
#address-cells = <1>;
#size-cells = <0>;
scmi_iomuxc: protocol@19 {
compatible = "fsl,imx95-scmi-pinctrl";
reg = <0x19>;
};
};
};
};
&scmi_iomuxc {
pinctrl_tpm3: tpm3grp {
fsl,pins = <
IMX95_PAD_GPIO_IO12__TPM3_CH2(0x51e)
>;
};
};
This patchset has been tested on i.MX95-19x19-EVK board.
Signed-off-by: Peng Fan <peng.fan@nxp.com>
---
Changes in v8:
- For the devm_x, I still keep as it is to follow current SCMI style. In
future we may follow cleanup.h, but it is not this patchset's goal.
- Apply Cristian's change, and add tag
- _pin -> pin to drop underscore
- Add headers per Andy's comments
- Drop casting for func->groups
- Minor update for coding style
- Link to v7: https://lore.kernel.org/r/20240402-pinctrl-scmi-v7-0-3ea519d12cf7@nxp.com
Changes in v7:
- Hope I not miss any comments. If any missed, please forgive. Since
i.MX95 SCMI firmware not support all the pinctrl features, I could only
do limited test.
- Version set to 0x10000
- Drop scmi_msg_func_set
- Use get_all to replace flag[0,1], not support flag 2 as of now.
- Add settings_get_one and settings_get_all ops to support get_all[false, true]
- PINCTRL_SET_PERMISSIONS is not included in this patchset
- Bail out if nr_pins is 0
- Add check nr_functions and nr_groups if they are 0.
- ext_name_flag changed to bool type
- Drop unrelated comment
- Use a central function for pin request and free
- Coding style optimization
- Use pinfunction to replace scmi_pinctrl_funcs
- For the devm_x APIs comments from Andy, I not update in the x/arm_scmi/pinctrl.c,
because it is correct usage.
- For included headers, I keep not change. I try to follow 80 max chars
for scmi driver, but with a few lines still exceed.
- Link to v6: https://lore.kernel.org/r/20240323-pinctrl-scmi-v6-0-a895243257c0@nxp.com
Changes in v6:
- Update pinctrl driver following ARM SCMI 3.2 public release
- Addressed Dan's comments, and followed Dan's suggestions, thanks.
- Dropped R-b/T-b in patch 3/4 and patch 4/4,
- Link to v5: https://lore.kernel.org/r/20240314-pinctrl-scmi-v5-0-b19576e557f2@nxp.com
Changes in v5:
- Rebased to linux-next next-20240313
- Link to v4: https://lore.kernel.org/r/20240223-pinctrl-scmi-v4-0-10eb5a379274@nxp.com
Changes in v4:
- Rebased to next-20240222
- Drop pinctrl-scmi-imx and compatible patches in V3
- Add T-b and R-b collected from v3
- Link to v3: https://lore.kernel.org/r/20240121-pinctrl-scmi-v3-0-8d94ba79dca8@nxp.com
Changes in v3:
- Add R-b for dt-binding patch
- Use 80 chars per line to align with other scmi drivers
- Add pinctrl_scmi_alloc_configs pinctrl_scmi_free_configs to replace
driver global config_value and config_type array to avoid in parrell
access issue. When num_configs is larger than 4, use alloc, else use
stack.
- Drop the separate MAITAINERS entry for firmware scmi pinctrl
- Use enum type, not u8 when referring the scmi or generic pin conf type
- Drop scmi_pinctrl_config_get_all which is not used at all for now.
- Update copyright year to 2024
- Move the enum scmi_pinctrl_conf_type above pinctrl_proto_ops for consistency
- Link to v2: https://lore.kernel.org/r/20240104-pinctrl-scmi-v2-0-a9bd86ab5a84@nxp.com
Changes in v2:
Added comments, and added R-b for Patch 1
Moved the compatile string and i.MX patch to the end, marked NOT APPLY
Patchset based on lore.kernel.org/all/20231221151129.325749-1-cristian.marussi@arm.com/
Addressed the binding doc issue, dropped i.MX content.
For the firmware pinctrl scmi driver, addressed the comments from Cristian
For the pinctrl scmi driver, addressed comments from Cristian
For the i.MX95 OEM stuff, I not have good idea, expect using compatbile
string. Maybe the firmware public an protocol attribute to indicate it is
VENDOR stuff or NXP use a new protocol id, not 0x19. But I think
current pinctrl-scmi.c not able to support OEM config, should we extend
it with some method? Anyway if patch 1-4 is good enough, they could
be picked up first.
Since I am only able to test the patch on i.MX95 which not support
geneirc pinconf, only OEM configs are tested in my side.
---
Peng Fan (4):
firmware: arm_scmi: introduce helper get_max_msg_size
dt-bindings: firmware: arm,scmi: support pinctrl protocol
firmware: arm_scmi: Add SCMI v3.2 pincontrol protocol basic support
pinctrl: Implementation of the generic scmi-pinctrl driver
.../devicetree/bindings/firmware/arm,scmi.yaml | 50 ++
MAINTAINERS | 1 +
drivers/firmware/arm_scmi/Makefile | 3 +-
drivers/firmware/arm_scmi/driver.c | 17 +
drivers/firmware/arm_scmi/pinctrl.c | 916 +++++++++++++++++++++
drivers/firmware/arm_scmi/protocols.h | 3 +
drivers/pinctrl/Kconfig | 11 +
drivers/pinctrl/Makefile | 1 +
drivers/pinctrl/pinctrl-scmi.c | 564 +++++++++++++
include/linux/scmi_protocol.h | 84 ++
10 files changed, 1649 insertions(+), 1 deletion(-)
---
base-commit: 70ca90deeca172d343bd18bf7fb2f992214c23c0
change-id: 20231215-pinctrl-scmi-4c5b0374f4c6
Best regards,
--
Peng Fan <peng.fan@nxp.com>
^ permalink raw reply
* [PATCH v8 1/4] firmware: arm_scmi: introduce helper get_max_msg_size
From: Peng Fan (OSS) @ 2024-04-05 1:59 UTC (permalink / raw)
To: Sudeep Holla, Cristian Marussi, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, Linus Walleij, Dan Carpenter
Cc: Andy Shevchenko, linux-arm-kernel, linux-kernel, devicetree,
linux-gpio, Peng Fan
In-Reply-To: <20240405-pinctrl-scmi-v8-0-5fc8e33871bf@nxp.com>
From: Peng Fan <peng.fan@nxp.com>
When Agent sending data to SCMI server, the Agent driver could check
the size to avoid protocol buffer overflow. So introduce the helper
get_max_msg_size.
Reviewed-by: Cristian Marussi <cristian.marussi@arm.com>
Signed-off-by: Peng Fan <peng.fan@nxp.com>
---
drivers/firmware/arm_scmi/driver.c | 15 +++++++++++++++
drivers/firmware/arm_scmi/protocols.h | 2 ++
2 files changed, 17 insertions(+)
diff --git a/drivers/firmware/arm_scmi/driver.c b/drivers/firmware/arm_scmi/driver.c
index 2709598f3008..415e6f510057 100644
--- a/drivers/firmware/arm_scmi/driver.c
+++ b/drivers/firmware/arm_scmi/driver.c
@@ -1488,6 +1488,20 @@ static int scmi_common_extended_name_get(const struct scmi_protocol_handle *ph,
return ret;
}
+/**
+ * scmi_common_get_max_msg_size - Get maximum message size
+ * @ph: A protocol handle reference.
+ *
+ * Return: Maximum message size for the current protocol.
+ */
+static int scmi_common_get_max_msg_size(const struct scmi_protocol_handle *ph)
+{
+ const struct scmi_protocol_instance *pi = ph_to_pi(ph);
+ struct scmi_info *info = handle_to_scmi_info(pi->handle);
+
+ return info->desc->max_msg_size;
+}
+
/**
* struct scmi_iterator - Iterator descriptor
* @msg: A reference to the message TX buffer; filled by @prepare_message with
@@ -1799,6 +1813,7 @@ static int scmi_protocol_msg_check(const struct scmi_protocol_handle *ph,
static const struct scmi_proto_helpers_ops helpers_ops = {
.extended_name_get = scmi_common_extended_name_get,
+ .get_max_msg_size = scmi_common_get_max_msg_size,
.iter_response_init = scmi_iterator_init,
.iter_response_run = scmi_iterator_run,
.protocol_msg_check = scmi_protocol_msg_check,
diff --git a/drivers/firmware/arm_scmi/protocols.h b/drivers/firmware/arm_scmi/protocols.h
index 317d3fb32676..3e91536a77a3 100644
--- a/drivers/firmware/arm_scmi/protocols.h
+++ b/drivers/firmware/arm_scmi/protocols.h
@@ -258,6 +258,7 @@ struct scmi_fc_info {
* @fastchannel_init: A common helper used to initialize FC descriptors by
* gathering FC descriptions from the SCMI platform server.
* @fastchannel_db_ring: A common helper to ring a FC doorbell.
+ * @get_max_msg_size: A common helper to get the maximum message size.
*/
struct scmi_proto_helpers_ops {
int (*extended_name_get)(const struct scmi_protocol_handle *ph,
@@ -277,6 +278,7 @@ struct scmi_proto_helpers_ops {
struct scmi_fc_db_info **p_db,
u32 *rate_limit);
void (*fastchannel_db_ring)(struct scmi_fc_db_info *db);
+ int (*get_max_msg_size)(const struct scmi_protocol_handle *ph);
};
/**
--
2.37.1
^ permalink raw reply related
* [PATCH v8 2/4] dt-bindings: firmware: arm,scmi: support pinctrl protocol
From: Peng Fan (OSS) @ 2024-04-05 1:59 UTC (permalink / raw)
To: Sudeep Holla, Cristian Marussi, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, Linus Walleij, Dan Carpenter
Cc: Andy Shevchenko, linux-arm-kernel, linux-kernel, devicetree,
linux-gpio, Peng Fan
In-Reply-To: <20240405-pinctrl-scmi-v8-0-5fc8e33871bf@nxp.com>
From: Peng Fan <peng.fan@nxp.com>
Add SCMI v3.2 pinctrl protocol bindings and example.
Reviewed-by: Rob Herring <robh@kernel.org>
Reviewed-by: Linus Walleij <linus.walleij@linaro.org>
Signed-off-by: Peng Fan <peng.fan@nxp.com>
---
.../devicetree/bindings/firmware/arm,scmi.yaml | 50 ++++++++++++++++++++++
1 file changed, 50 insertions(+)
diff --git a/Documentation/devicetree/bindings/firmware/arm,scmi.yaml b/Documentation/devicetree/bindings/firmware/arm,scmi.yaml
index 4591523b51a0..e9d3f043c4ed 100644
--- a/Documentation/devicetree/bindings/firmware/arm,scmi.yaml
+++ b/Documentation/devicetree/bindings/firmware/arm,scmi.yaml
@@ -247,6 +247,37 @@ properties:
reg:
const: 0x18
+ protocol@19:
+ type: object
+ allOf:
+ - $ref: '#/$defs/protocol-node'
+ - $ref: /schemas/pinctrl/pinctrl.yaml
+
+ unevaluatedProperties: false
+
+ properties:
+ reg:
+ const: 0x19
+
+ patternProperties:
+ '-pins$':
+ type: object
+ allOf:
+ - $ref: /schemas/pinctrl/pincfg-node.yaml#
+ - $ref: /schemas/pinctrl/pinmux-node.yaml#
+ unevaluatedProperties: false
+
+ description:
+ A pin multiplexing sub-node describes how to configure a
+ set of pins in some desired function.
+ A single sub-node may define several pin configurations.
+ This sub-node is using the default pinctrl bindings to configure
+ pin multiplexing and using SCMI protocol to apply a specified
+ configuration.
+
+ required:
+ - reg
+
additionalProperties: false
$defs:
@@ -401,6 +432,25 @@ examples:
scmi_powercap: protocol@18 {
reg = <0x18>;
};
+
+ scmi_pinctrl: protocol@19 {
+ reg = <0x19>;
+
+ i2c2-pins {
+ groups = "g_i2c2_a", "g_i2c2_b";
+ function = "f_i2c2";
+ };
+
+ mdio-pins {
+ groups = "g_avb_mdio";
+ drive-strength = <24>;
+ };
+
+ keys_pins: keys-pins {
+ pins = "gpio_5_17", "gpio_5_20", "gpio_5_22", "gpio_2_1";
+ bias-pull-up;
+ };
+ };
};
};
--
2.37.1
^ permalink raw reply related
* [PATCH v8 3/4] firmware: arm_scmi: Add SCMI v3.2 pincontrol protocol basic support
From: Peng Fan (OSS) @ 2024-04-05 1:59 UTC (permalink / raw)
To: Sudeep Holla, Cristian Marussi, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, Linus Walleij, Dan Carpenter
Cc: Andy Shevchenko, linux-arm-kernel, linux-kernel, devicetree,
linux-gpio, Peng Fan, Oleksii Moisieiev
In-Reply-To: <20240405-pinctrl-scmi-v8-0-5fc8e33871bf@nxp.com>
From: Peng Fan <peng.fan@nxp.com>
Add basic implementation of the SCMI v3.2 pincontrol protocol.
Co-developed-by: Oleksii Moisieiev <oleksii_moisieiev@epam.com>
Signed-off-by: Oleksii Moisieiev <oleksii_moisieiev@epam.com>
Co-developed-by: Cristian Marussi <cristian.marussi@arm.com>
Signed-off-by: Cristian Marussi <cristian.marussi@arm.com>
Signed-off-by: Peng Fan <peng.fan@nxp.com>
---
drivers/firmware/arm_scmi/Makefile | 3 +-
drivers/firmware/arm_scmi/driver.c | 2 +
drivers/firmware/arm_scmi/pinctrl.c | 916 ++++++++++++++++++++++++++++++++++
drivers/firmware/arm_scmi/protocols.h | 1 +
include/linux/scmi_protocol.h | 84 ++++
5 files changed, 1005 insertions(+), 1 deletion(-)
diff --git a/drivers/firmware/arm_scmi/Makefile b/drivers/firmware/arm_scmi/Makefile
index a7bc4796519c..fd59f58ce8a2 100644
--- a/drivers/firmware/arm_scmi/Makefile
+++ b/drivers/firmware/arm_scmi/Makefile
@@ -10,7 +10,8 @@ scmi-transport-$(CONFIG_ARM_SCMI_TRANSPORT_SMC) += smc.o
scmi-transport-$(CONFIG_ARM_SCMI_HAVE_MSG) += msg.o
scmi-transport-$(CONFIG_ARM_SCMI_TRANSPORT_VIRTIO) += virtio.o
scmi-transport-$(CONFIG_ARM_SCMI_TRANSPORT_OPTEE) += optee.o
-scmi-protocols-y = base.o clock.o perf.o power.o reset.o sensors.o system.o voltage.o powercap.o
+scmi-protocols-y := base.o clock.o perf.o power.o reset.o sensors.o system.o voltage.o powercap.o
+scmi-protocols-y += pinctrl.o
scmi-module-objs := $(scmi-driver-y) $(scmi-protocols-y) $(scmi-transport-y)
obj-$(CONFIG_ARM_SCMI_PROTOCOL) += scmi-core.o
diff --git a/drivers/firmware/arm_scmi/driver.c b/drivers/firmware/arm_scmi/driver.c
index 415e6f510057..ac2d4b19727c 100644
--- a/drivers/firmware/arm_scmi/driver.c
+++ b/drivers/firmware/arm_scmi/driver.c
@@ -3142,6 +3142,7 @@ static int __init scmi_driver_init(void)
scmi_voltage_register();
scmi_system_register();
scmi_powercap_register();
+ scmi_pinctrl_register();
return platform_driver_register(&scmi_driver);
}
@@ -3159,6 +3160,7 @@ static void __exit scmi_driver_exit(void)
scmi_voltage_unregister();
scmi_system_unregister();
scmi_powercap_unregister();
+ scmi_pinctrl_unregister();
scmi_transports_exit();
diff --git a/drivers/firmware/arm_scmi/pinctrl.c b/drivers/firmware/arm_scmi/pinctrl.c
new file mode 100644
index 000000000000..a2a7f880d6a3
--- /dev/null
+++ b/drivers/firmware/arm_scmi/pinctrl.c
@@ -0,0 +1,916 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * System Control and Management Interface (SCMI) Pinctrl Protocol
+ *
+ * Copyright (C) 2024 EPAM
+ * Copyright 2024 NXP
+ */
+
+#include <asm/byteorder.h>
+#include <linux/bits.h>
+#include <linux/bitfield.h>
+#include <linux/device.h>
+#include <linux/module.h>
+#include <linux/scmi_protocol.h>
+#include <linux/slab.h>
+#include <linux/string.h>
+#include <linux/types.h>
+
+#include "common.h"
+#include "protocols.h"
+
+/* Updated only after ALL the mandatory features for that version are merged */
+#define SCMI_PROTOCOL_SUPPORTED_VERSION 0x10000
+
+#define GET_GROUPS_NR(x) le32_get_bits((x), GENMASK(31, 16))
+#define GET_PINS_NR(x) le32_get_bits((x), GENMASK(15, 0))
+#define GET_FUNCTIONS_NR(x) le32_get_bits((x), GENMASK(15, 0))
+
+#define EXT_NAME_FLAG(x) le32_get_bits((x), BIT(31))
+#define NUM_ELEMS(x) le32_get_bits((x), GENMASK(15, 0))
+
+#define REMAINING(x) le32_get_bits((x), GENMASK(31, 16))
+#define RETURNED(x) le32_get_bits((x), GENMASK(11, 0))
+
+#define CONFIG_FLAG_MASK GENMASK(19, 18)
+#define SELECTOR_MASK GENMASK(17, 16)
+#define SKIP_CONFIGS_MASK GENMASK(15, 8)
+#define CONFIG_TYPE_MASK GENMASK(7, 0)
+
+enum scmi_pinctrl_protocol_cmd {
+ PINCTRL_ATTRIBUTES = 0x3,
+ PINCTRL_LIST_ASSOCIATIONS = 0x4,
+ PINCTRL_SETTINGS_GET = 0x5,
+ PINCTRL_SETTINGS_CONFIGURE = 0x6,
+ PINCTRL_REQUEST = 0x7,
+ PINCTRL_RELEASE = 0x8,
+ PINCTRL_NAME_GET = 0x9,
+ PINCTRL_SET_PERMISSIONS = 0xa,
+};
+
+struct scmi_msg_settings_conf {
+ __le32 identifier;
+ __le32 function_id;
+ __le32 attributes;
+ __le32 configs[];
+};
+
+struct scmi_msg_settings_get {
+ __le32 identifier;
+ __le32 attributes;
+};
+
+struct scmi_resp_settings_get {
+ __le32 function_selected;
+ __le32 num_configs;
+ __le32 configs[];
+};
+
+struct scmi_msg_pinctrl_protocol_attributes {
+ __le32 attributes_low;
+ __le32 attributes_high;
+};
+
+struct scmi_msg_pinctrl_attributes {
+ __le32 identifier;
+ __le32 flags;
+};
+
+struct scmi_resp_pinctrl_attributes {
+ __le32 attributes;
+ u8 name[SCMI_SHORT_NAME_MAX_SIZE];
+};
+
+struct scmi_msg_pinctrl_list_assoc {
+ __le32 identifier;
+ __le32 flags;
+ __le32 index;
+};
+
+struct scmi_resp_pinctrl_list_assoc {
+ __le32 flags;
+ __le16 array[];
+};
+
+struct scmi_msg_request {
+ __le32 identifier;
+ __le32 flags;
+};
+
+struct scmi_group_info {
+ char name[SCMI_MAX_STR_SIZE];
+ bool present;
+ u32 *group_pins;
+ u32 nr_pins;
+};
+
+struct scmi_function_info {
+ char name[SCMI_MAX_STR_SIZE];
+ bool present;
+ u32 *groups;
+ u32 nr_groups;
+};
+
+struct scmi_pin_info {
+ char name[SCMI_MAX_STR_SIZE];
+ bool present;
+};
+
+struct scmi_pinctrl_info {
+ u32 version;
+ int nr_groups;
+ int nr_functions;
+ int nr_pins;
+ struct scmi_group_info *groups;
+ struct scmi_function_info *functions;
+ struct scmi_pin_info *pins;
+};
+
+static int scmi_pinctrl_attributes_get(const struct scmi_protocol_handle *ph,
+ struct scmi_pinctrl_info *pi)
+{
+ int ret;
+ struct scmi_xfer *t;
+ struct scmi_msg_pinctrl_protocol_attributes *attr;
+
+ ret = ph->xops->xfer_get_init(ph, PROTOCOL_ATTRIBUTES, 0, sizeof(*attr), &t);
+ if (ret)
+ return ret;
+
+ attr = t->rx.buf;
+
+ ret = ph->xops->do_xfer(ph, t);
+ if (!ret) {
+ pi->nr_functions = GET_FUNCTIONS_NR(attr->attributes_high);
+ pi->nr_groups = GET_GROUPS_NR(attr->attributes_low);
+ pi->nr_pins = GET_PINS_NR(attr->attributes_low);
+ if (pi->nr_pins == 0) {
+ dev_warn(ph->dev, "returned zero pins\n");
+ ret = -EINVAL;
+ }
+ }
+
+ ph->xops->xfer_put(ph, t);
+ return ret;
+}
+
+static int scmi_pinctrl_count_get(const struct scmi_protocol_handle *ph,
+ enum scmi_pinctrl_selector_type type)
+{
+ struct scmi_pinctrl_info *pi = ph->get_priv(ph);
+
+ switch (type) {
+ case PIN_TYPE:
+ return pi->nr_pins;
+ case GROUP_TYPE:
+ return pi->nr_groups;
+ case FUNCTION_TYPE:
+ return pi->nr_functions;
+ default:
+ return -EINVAL;
+ }
+}
+
+static int scmi_pinctrl_validate_id(const struct scmi_protocol_handle *ph,
+ u32 selector,
+ enum scmi_pinctrl_selector_type type)
+{
+ int value;
+
+ value = scmi_pinctrl_count_get(ph, type);
+ if (value < 0)
+ return value;
+
+ if (selector >= value || value == 0)
+ return -EINVAL;
+
+ return 0;
+}
+
+static int scmi_pinctrl_attributes(const struct scmi_protocol_handle *ph,
+ enum scmi_pinctrl_selector_type type,
+ u32 selector, char *name,
+ u32 *n_elems)
+{
+ int ret;
+ struct scmi_xfer *t;
+ struct scmi_msg_pinctrl_attributes *tx;
+ struct scmi_resp_pinctrl_attributes *rx;
+ bool ext_name_flag;
+
+ if (!name)
+ return -EINVAL;
+
+ ret = scmi_pinctrl_validate_id(ph, selector, type);
+ if (ret)
+ return ret;
+
+ ret = ph->xops->xfer_get_init(ph, PINCTRL_ATTRIBUTES, sizeof(*tx),
+ sizeof(*rx), &t);
+ if (ret)
+ return ret;
+
+ tx = t->tx.buf;
+ rx = t->rx.buf;
+ tx->identifier = cpu_to_le32(selector);
+ tx->flags = cpu_to_le32(type);
+
+ ret = ph->xops->do_xfer(ph, t);
+ if (!ret) {
+ if (n_elems)
+ *n_elems = NUM_ELEMS(rx->attributes);
+
+ strscpy(name, rx->name, SCMI_SHORT_NAME_MAX_SIZE);
+
+ ext_name_flag = !!EXT_NAME_FLAG(rx->attributes);
+ }
+
+ ph->xops->xfer_put(ph, t);
+
+ if (ret)
+ return ret;
+ /*
+ * If supported overwrite short name with the extended one;
+ * on error just carry on and use already provided short name.
+ */
+ if (ext_name_flag)
+ ret = ph->hops->extended_name_get(ph, PINCTRL_NAME_GET,
+ selector, (u32 *)&type, name,
+ SCMI_MAX_STR_SIZE);
+ return ret;
+}
+
+struct scmi_pinctrl_ipriv {
+ u32 selector;
+ enum scmi_pinctrl_selector_type type;
+ u32 *array;
+};
+
+static void iter_pinctrl_assoc_prepare_message(void *message,
+ u32 desc_index,
+ const void *priv)
+{
+ struct scmi_msg_pinctrl_list_assoc *msg = message;
+ const struct scmi_pinctrl_ipriv *p = priv;
+
+ msg->identifier = cpu_to_le32(p->selector);
+ msg->flags = cpu_to_le32(p->type);
+ msg->index = cpu_to_le32(desc_index);
+}
+
+static int iter_pinctrl_assoc_update_state(struct scmi_iterator_state *st,
+ const void *response, void *priv)
+{
+ const struct scmi_resp_pinctrl_list_assoc *r = response;
+
+ st->num_returned = RETURNED(r->flags);
+ st->num_remaining = REMAINING(r->flags);
+
+ return 0;
+}
+
+static int
+iter_pinctrl_assoc_process_response(const struct scmi_protocol_handle *ph,
+ const void *response,
+ struct scmi_iterator_state *st, void *priv)
+{
+ const struct scmi_resp_pinctrl_list_assoc *r = response;
+ struct scmi_pinctrl_ipriv *p = priv;
+
+ p->array[st->desc_index + st->loop_idx] =
+ le16_to_cpu(r->array[st->loop_idx]);
+
+ return 0;
+}
+
+static int scmi_pinctrl_list_associations(const struct scmi_protocol_handle *ph,
+ u32 selector,
+ enum scmi_pinctrl_selector_type type,
+ u16 size, u32 *array)
+{
+ int ret;
+ void *iter;
+ struct scmi_iterator_ops ops = {
+ .prepare_message = iter_pinctrl_assoc_prepare_message,
+ .update_state = iter_pinctrl_assoc_update_state,
+ .process_response = iter_pinctrl_assoc_process_response,
+ };
+ struct scmi_pinctrl_ipriv ipriv = {
+ .selector = selector,
+ .type = type,
+ .array = array,
+ };
+
+ if (!array || !size || type == PIN_TYPE)
+ return -EINVAL;
+
+ ret = scmi_pinctrl_validate_id(ph, selector, type);
+ if (ret)
+ return ret;
+
+ iter = ph->hops->iter_response_init(ph, &ops, size,
+ PINCTRL_LIST_ASSOCIATIONS,
+ sizeof(struct scmi_msg_pinctrl_list_assoc),
+ &ipriv);
+ if (IS_ERR(iter))
+ return PTR_ERR(iter);
+
+ return ph->hops->iter_response_run(iter);
+}
+
+struct scmi_settings_get_ipriv {
+ u32 selector;
+ enum scmi_pinctrl_selector_type type;
+ bool get_all;
+ unsigned int *nr_configs;
+ enum scmi_pinctrl_conf_type *config_types;
+ u32 *config_values;
+};
+
+static void
+iter_pinctrl_settings_get_prepare_message(void *message, u32 desc_index,
+ const void *priv)
+{
+ struct scmi_msg_settings_get *msg = message;
+ const struct scmi_settings_get_ipriv *p = priv;
+ u32 attributes;
+
+ attributes = FIELD_PREP(SELECTOR_MASK, p->type);
+
+ if (p->get_all) {
+ attributes |= FIELD_PREP(CONFIG_FLAG_MASK, 1) |
+ FIELD_PREP(SKIP_CONFIGS_MASK, desc_index);
+ } else {
+ attributes |= FIELD_PREP(CONFIG_TYPE_MASK, p->config_types[0]);
+ }
+
+ msg->attributes = cpu_to_le32(attributes);
+ msg->identifier = cpu_to_le32(p->selector);
+}
+
+static int
+iter_pinctrl_settings_get_update_state(struct scmi_iterator_state *st,
+ const void *response, void *priv)
+{
+ const struct scmi_resp_settings_get *r = response;
+ struct scmi_settings_get_ipriv *p = priv;
+
+ if (p->get_all) {
+ st->num_returned = le32_get_bits(r->num_configs, GENMASK(7, 0));
+ st->num_remaining = le32_get_bits(r->num_configs, GENMASK(31, 24));
+ } else {
+ st->num_returned = 1;
+ st->num_remaining = 0;
+ }
+
+ return 0;
+}
+
+static int
+iter_pinctrl_settings_get_process_response(const struct scmi_protocol_handle *ph,
+ const void *response,
+ struct scmi_iterator_state *st,
+ void *priv)
+{
+ const struct scmi_resp_settings_get *r = response;
+ struct scmi_settings_get_ipriv *p = priv;
+ u32 type = le32_get_bits(r->configs[st->loop_idx * 2], GENMASK(7, 0));
+ u32 val = le32_to_cpu(r->configs[st->loop_idx * 2 + 1]);
+
+ if (p->get_all) {
+ p->config_types[st->desc_index + st->loop_idx] = type;
+ } else {
+ if (p->config_types[0] != type)
+ return -EINVAL;
+ }
+
+ p->config_values[st->desc_index + st->loop_idx] = val;
+ ++*p->nr_configs;
+
+ return 0;
+}
+
+static int
+scmi_pinctrl_settings_get(const struct scmi_protocol_handle *ph, u32 selector,
+ enum scmi_pinctrl_selector_type type,
+ unsigned int *nr_configs,
+ enum scmi_pinctrl_conf_type *config_types,
+ u32 *config_values)
+{
+ int ret;
+ void *iter;
+ unsigned int max_configs = *nr_configs;
+ struct scmi_iterator_ops ops = {
+ .prepare_message = iter_pinctrl_settings_get_prepare_message,
+ .update_state = iter_pinctrl_settings_get_update_state,
+ .process_response = iter_pinctrl_settings_get_process_response,
+ };
+ struct scmi_settings_get_ipriv ipriv = {
+ .selector = selector,
+ .type = type,
+ .get_all = (max_configs > 1),
+ .nr_configs = nr_configs,
+ .config_types = config_types,
+ .config_values = config_values,
+ };
+
+ if (!config_types || !config_values || type == FUNCTION_TYPE)
+ return -EINVAL;
+
+ ret = scmi_pinctrl_validate_id(ph, selector, type);
+ if (ret)
+ return ret;
+
+ /* Prepare to count returned configs */
+ *nr_configs = 0;
+ iter = ph->hops->iter_response_init(ph, &ops, max_configs,
+ PINCTRL_SETTINGS_GET,
+ sizeof(struct scmi_msg_settings_get),
+ &ipriv);
+ if (IS_ERR(iter))
+ return PTR_ERR(iter);
+
+ return ph->hops->iter_response_run(iter);
+}
+
+static int scmi_pinctrl_settings_get_one(const struct scmi_protocol_handle *ph,
+ u32 selector,
+ enum scmi_pinctrl_selector_type type,
+ enum scmi_pinctrl_conf_type config_type,
+ u32 *config_value)
+{
+ unsigned int nr_configs = 1;
+
+ return scmi_pinctrl_settings_get(ph, selector, type, &nr_configs,
+ &config_type, config_value);
+}
+
+static int scmi_pinctrl_settings_get_all(const struct scmi_protocol_handle *ph,
+ u32 selector,
+ enum scmi_pinctrl_selector_type type,
+ unsigned int *nr_configs,
+ enum scmi_pinctrl_conf_type *config_types,
+ u32 *config_values)
+{
+ if (!nr_configs || *nr_configs == 0)
+ return -EINVAL;
+
+ return scmi_pinctrl_settings_get(ph, selector, type, nr_configs,
+ config_types, config_values);
+}
+
+static int
+scmi_pinctrl_settings_conf(const struct scmi_protocol_handle *ph,
+ u32 selector,
+ enum scmi_pinctrl_selector_type type,
+ u32 nr_configs,
+ enum scmi_pinctrl_conf_type *config_type,
+ u32 *config_value)
+{
+ struct scmi_xfer *t;
+ struct scmi_msg_settings_conf *tx;
+ u32 attributes;
+ int ret, i;
+ u32 configs_in_chunk, conf_num = 0;
+ u32 chunk;
+ int max_msg_size = ph->hops->get_max_msg_size(ph);
+
+ if (!config_type || !config_value || type == FUNCTION_TYPE)
+ return -EINVAL;
+
+ ret = scmi_pinctrl_validate_id(ph, selector, type);
+ if (ret)
+ return ret;
+
+ configs_in_chunk = (max_msg_size - sizeof(*tx)) / (sizeof(__le32) * 2);
+ while (conf_num < nr_configs) {
+ chunk = (nr_configs - conf_num > configs_in_chunk) ?
+ configs_in_chunk : nr_configs - conf_num;
+
+ ret = ph->xops->xfer_get_init(ph, PINCTRL_SETTINGS_CONFIGURE,
+ sizeof(*tx) +
+ chunk * 2 * sizeof(__le32), 0, &t);
+ if (ret)
+ break;
+
+ tx = t->tx.buf;
+ tx->identifier = cpu_to_le32(selector);
+ tx->function_id = cpu_to_le32(0xFFFFFFFF);
+ attributes = FIELD_PREP(GENMASK(1, 0), type) |
+ FIELD_PREP(GENMASK(9, 2), chunk);
+ tx->attributes = cpu_to_le32(attributes);
+
+ for (i = 0; i < chunk; i++) {
+ tx->configs[i * 2] =
+ cpu_to_le32(config_type[conf_num + i]);
+ tx->configs[i * 2 + 1] =
+ cpu_to_le32(config_value[conf_num + i]);
+ }
+
+ ret = ph->xops->do_xfer(ph, t);
+
+ ph->xops->xfer_put(ph, t);
+
+ if (ret)
+ break;
+
+ conf_num += chunk;
+ }
+
+ return ret;
+}
+
+static int scmi_pinctrl_function_select(const struct scmi_protocol_handle *ph,
+ u32 group,
+ enum scmi_pinctrl_selector_type type,
+ u32 function_id)
+{
+ int ret;
+ struct scmi_xfer *t;
+ struct scmi_msg_settings_conf *tx;
+ u32 attributes;
+
+ ret = scmi_pinctrl_validate_id(ph, group, type);
+ if (ret)
+ return ret;
+
+ ret = ph->xops->xfer_get_init(ph, PINCTRL_SETTINGS_CONFIGURE,
+ sizeof(*tx), 0, &t);
+ if (ret)
+ return ret;
+
+ tx = t->tx.buf;
+ tx->identifier = cpu_to_le32(group);
+ tx->function_id = cpu_to_le32(function_id);
+ attributes = FIELD_PREP(GENMASK(1, 0), type) | BIT(10);
+ tx->attributes = cpu_to_le32(attributes);
+
+ ret = ph->xops->do_xfer(ph, t);
+ ph->xops->xfer_put(ph, t);
+
+ return ret;
+}
+
+static int scmi_pinctrl_request_free(const struct scmi_protocol_handle *ph,
+ u32 identifier,
+ enum scmi_pinctrl_selector_type type,
+ enum scmi_pinctrl_protocol_cmd cmd)
+{
+ int ret;
+ struct scmi_xfer *t;
+ struct scmi_msg_request *tx;
+
+ if (type == FUNCTION_TYPE)
+ return -EINVAL;
+
+ if (cmd != PINCTRL_REQUEST && cmd != PINCTRL_RELEASE)
+ return -EINVAL;
+
+ ret = scmi_pinctrl_validate_id(ph, identifier, type);
+ if (ret)
+ return ret;
+
+ ret = ph->xops->xfer_get_init(ph, cmd, sizeof(*tx), 0, &t);
+ if (ret)
+ return ret;
+
+ tx = t->tx.buf;
+ tx->identifier = cpu_to_le32(identifier);
+ tx->flags = cpu_to_le32(type);
+
+ ret = ph->xops->do_xfer(ph, t);
+ ph->xops->xfer_put(ph, t);
+
+ return ret;
+}
+
+static int scmi_pinctrl_pin_request(const struct scmi_protocol_handle *ph,
+ u32 pin)
+{
+ return scmi_pinctrl_request_free(ph, pin, PIN_TYPE, PINCTRL_REQUEST);
+}
+
+static int scmi_pinctrl_pin_free(const struct scmi_protocol_handle *ph, u32 pin)
+{
+ return scmi_pinctrl_request_free(ph, pin, PIN_TYPE, PINCTRL_RELEASE);
+}
+
+static int scmi_pinctrl_get_group_info(const struct scmi_protocol_handle *ph,
+ u32 selector,
+ struct scmi_group_info *group)
+{
+ int ret;
+
+ ret = scmi_pinctrl_attributes(ph, GROUP_TYPE, selector, group->name,
+ &group->nr_pins);
+ if (ret)
+ return ret;
+
+ if (!group->nr_pins) {
+ dev_err(ph->dev, "Group %d has 0 elements", selector);
+ return -ENODATA;
+ }
+
+ group->group_pins = kmalloc_array(group->nr_pins,
+ sizeof(*group->group_pins),
+ GFP_KERNEL);
+ if (!group->group_pins)
+ return -ENOMEM;
+
+ ret = scmi_pinctrl_list_associations(ph, selector, GROUP_TYPE,
+ group->nr_pins, group->group_pins);
+ if (ret) {
+ kfree(group->group_pins);
+ return ret;
+ }
+
+ group->present = true;
+ return 0;
+}
+
+static int scmi_pinctrl_get_group_name(const struct scmi_protocol_handle *ph,
+ u32 selector, const char **name)
+{
+ struct scmi_pinctrl_info *pi = ph->get_priv(ph);
+
+ if (!name)
+ return -EINVAL;
+
+ if (selector >= pi->nr_groups || pi->nr_groups == 0)
+ return -EINVAL;
+
+ if (!pi->groups[selector].present) {
+ int ret;
+
+ ret = scmi_pinctrl_get_group_info(ph, selector,
+ &pi->groups[selector]);
+ if (ret)
+ return ret;
+ }
+
+ *name = pi->groups[selector].name;
+
+ return 0;
+}
+
+static int scmi_pinctrl_group_pins_get(const struct scmi_protocol_handle *ph,
+ u32 selector, const u32 **pins,
+ u32 *nr_pins)
+{
+ struct scmi_pinctrl_info *pi = ph->get_priv(ph);
+
+ if (!pins || !nr_pins)
+ return -EINVAL;
+
+ if (selector >= pi->nr_groups || pi->nr_groups == 0)
+ return -EINVAL;
+
+ if (!pi->groups[selector].present) {
+ int ret;
+
+ ret = scmi_pinctrl_get_group_info(ph, selector,
+ &pi->groups[selector]);
+ if (ret)
+ return ret;
+ }
+
+ *pins = pi->groups[selector].group_pins;
+ *nr_pins = pi->groups[selector].nr_pins;
+
+ return 0;
+}
+
+static int scmi_pinctrl_get_function_info(const struct scmi_protocol_handle *ph,
+ u32 selector,
+ struct scmi_function_info *func)
+{
+ int ret;
+
+ ret = scmi_pinctrl_attributes(ph, FUNCTION_TYPE, selector, func->name,
+ &func->nr_groups);
+ if (ret)
+ return ret;
+
+ if (!func->nr_groups) {
+ dev_err(ph->dev, "Function %d has 0 elements", selector);
+ return -ENODATA;
+ }
+
+ func->groups = kmalloc_array(func->nr_groups, sizeof(*func->groups),
+ GFP_KERNEL);
+ if (!func->groups)
+ return -ENOMEM;
+
+ ret = scmi_pinctrl_list_associations(ph, selector, FUNCTION_TYPE,
+ func->nr_groups, func->groups);
+ if (ret) {
+ kfree(func->groups);
+ return ret;
+ }
+
+ func->present = true;
+ return 0;
+}
+
+static int scmi_pinctrl_get_function_name(const struct scmi_protocol_handle *ph,
+ u32 selector, const char **name)
+{
+ struct scmi_pinctrl_info *pi = ph->get_priv(ph);
+
+ if (!name)
+ return -EINVAL;
+
+ if (selector >= pi->nr_functions || pi->nr_functions == 0)
+ return -EINVAL;
+
+ if (!pi->functions[selector].present) {
+ int ret;
+
+ ret = scmi_pinctrl_get_function_info(ph, selector,
+ &pi->functions[selector]);
+ if (ret)
+ return ret;
+ }
+
+ *name = pi->functions[selector].name;
+ return 0;
+}
+
+static int
+scmi_pinctrl_function_groups_get(const struct scmi_protocol_handle *ph,
+ u32 selector, u32 *nr_groups,
+ const u32 **groups)
+{
+ struct scmi_pinctrl_info *pi = ph->get_priv(ph);
+
+ if (!groups || !nr_groups)
+ return -EINVAL;
+
+ if (selector >= pi->nr_functions || pi->nr_functions == 0)
+ return -EINVAL;
+
+ if (!pi->functions[selector].present) {
+ int ret;
+
+ ret = scmi_pinctrl_get_function_info(ph, selector,
+ &pi->functions[selector]);
+ if (ret)
+ return ret;
+ }
+
+ *groups = pi->functions[selector].groups;
+ *nr_groups = pi->functions[selector].nr_groups;
+
+ return 0;
+}
+
+static int scmi_pinctrl_mux_set(const struct scmi_protocol_handle *ph,
+ u32 selector, u32 group)
+{
+ return scmi_pinctrl_function_select(ph, group, GROUP_TYPE, selector);
+}
+
+static int scmi_pinctrl_get_pin_info(const struct scmi_protocol_handle *ph,
+ u32 selector, struct scmi_pin_info *pin)
+{
+ int ret;
+
+ if (!pin)
+ return -EINVAL;
+
+ ret = scmi_pinctrl_attributes(ph, PIN_TYPE, selector, pin->name, NULL);
+ if (ret)
+ return ret;
+
+ pin->present = true;
+ return 0;
+}
+
+static int scmi_pinctrl_get_pin_name(const struct scmi_protocol_handle *ph,
+ u32 selector, const char **name)
+{
+ struct scmi_pinctrl_info *pi = ph->get_priv(ph);
+
+ if (!name)
+ return -EINVAL;
+
+ if (selector >= pi->nr_pins)
+ return -EINVAL;
+
+ if (!pi->pins[selector].present) {
+ int ret;
+
+ ret = scmi_pinctrl_get_pin_info(ph, selector, &pi->pins[selector]);
+ if (ret)
+ return ret;
+ }
+
+ *name = pi->pins[selector].name;
+
+ return 0;
+}
+
+static int scmi_pinctrl_name_get(const struct scmi_protocol_handle *ph,
+ u32 selector,
+ enum scmi_pinctrl_selector_type type,
+ const char **name)
+{
+ switch (type) {
+ case PIN_TYPE:
+ return scmi_pinctrl_get_pin_name(ph, selector, name);
+ case GROUP_TYPE:
+ return scmi_pinctrl_get_group_name(ph, selector, name);
+ case FUNCTION_TYPE:
+ return scmi_pinctrl_get_function_name(ph, selector, name);
+ default:
+ return -EINVAL;
+ }
+}
+
+static const struct scmi_pinctrl_proto_ops pinctrl_proto_ops = {
+ .count_get = scmi_pinctrl_count_get,
+ .name_get = scmi_pinctrl_name_get,
+ .group_pins_get = scmi_pinctrl_group_pins_get,
+ .function_groups_get = scmi_pinctrl_function_groups_get,
+ .mux_set = scmi_pinctrl_mux_set,
+ .settings_get_one = scmi_pinctrl_settings_get_one,
+ .settings_get_all = scmi_pinctrl_settings_get_all,
+ .settings_conf = scmi_pinctrl_settings_conf,
+ .pin_request = scmi_pinctrl_pin_request,
+ .pin_free = scmi_pinctrl_pin_free,
+};
+
+static int scmi_pinctrl_protocol_init(const struct scmi_protocol_handle *ph)
+{
+ int ret;
+ u32 version;
+ struct scmi_pinctrl_info *pinfo;
+
+ ret = ph->xops->version_get(ph, &version);
+ if (ret)
+ return ret;
+
+ dev_dbg(ph->dev, "Pinctrl Version %d.%d\n",
+ PROTOCOL_REV_MAJOR(version), PROTOCOL_REV_MINOR(version));
+
+ pinfo = devm_kzalloc(ph->dev, sizeof(*pinfo), GFP_KERNEL);
+ if (!pinfo)
+ return -ENOMEM;
+
+ ret = scmi_pinctrl_attributes_get(ph, pinfo);
+ if (ret)
+ return ret;
+
+ pinfo->pins = devm_kcalloc(ph->dev, pinfo->nr_pins,
+ sizeof(*pinfo->pins), GFP_KERNEL);
+ if (!pinfo->pins)
+ return -ENOMEM;
+
+ pinfo->groups = devm_kcalloc(ph->dev, pinfo->nr_groups,
+ sizeof(*pinfo->groups), GFP_KERNEL);
+ if (!pinfo->groups)
+ return -ENOMEM;
+
+ pinfo->functions = devm_kcalloc(ph->dev, pinfo->nr_functions,
+ sizeof(*pinfo->functions), GFP_KERNEL);
+ if (!pinfo->functions)
+ return -ENOMEM;
+
+ pinfo->version = version;
+
+ return ph->set_priv(ph, pinfo, version);
+}
+
+static int scmi_pinctrl_protocol_deinit(const struct scmi_protocol_handle *ph)
+{
+ int i;
+ struct scmi_pinctrl_info *pi = ph->get_priv(ph);
+
+ /* Free groups_pins allocated in scmi_pinctrl_get_group_info */
+ for (i = 0; i < pi->nr_groups; i++) {
+ if (pi->groups[i].present) {
+ kfree(pi->groups[i].group_pins);
+ pi->groups[i].present = false;
+ }
+ }
+
+ /* Free groups allocated in scmi_pinctrl_get_function_info */
+ for (i = 0; i < pi->nr_functions; i++) {
+ if (pi->functions[i].present) {
+ kfree(pi->functions[i].groups);
+ pi->functions[i].present = false;
+ }
+ }
+
+ return 0;
+}
+
+static const struct scmi_protocol scmi_pinctrl = {
+ .id = SCMI_PROTOCOL_PINCTRL,
+ .owner = THIS_MODULE,
+ .instance_init = &scmi_pinctrl_protocol_init,
+ .instance_deinit = &scmi_pinctrl_protocol_deinit,
+ .ops = &pinctrl_proto_ops,
+ .supported_version = SCMI_PROTOCOL_SUPPORTED_VERSION,
+};
+DEFINE_SCMI_PROTOCOL_REGISTER_UNREGISTER(pinctrl, scmi_pinctrl)
diff --git a/drivers/firmware/arm_scmi/protocols.h b/drivers/firmware/arm_scmi/protocols.h
index 3e91536a77a3..c02cbfd2bb03 100644
--- a/drivers/firmware/arm_scmi/protocols.h
+++ b/drivers/firmware/arm_scmi/protocols.h
@@ -355,6 +355,7 @@ void __exit scmi_##name##_unregister(void) \
DECLARE_SCMI_REGISTER_UNREGISTER(base);
DECLARE_SCMI_REGISTER_UNREGISTER(clock);
DECLARE_SCMI_REGISTER_UNREGISTER(perf);
+DECLARE_SCMI_REGISTER_UNREGISTER(pinctrl);
DECLARE_SCMI_REGISTER_UNREGISTER(power);
DECLARE_SCMI_REGISTER_UNREGISTER(reset);
DECLARE_SCMI_REGISTER_UNREGISTER(sensors);
diff --git a/include/linux/scmi_protocol.h b/include/linux/scmi_protocol.h
index b807141acc14..5e75578706ce 100644
--- a/include/linux/scmi_protocol.h
+++ b/include/linux/scmi_protocol.h
@@ -737,6 +737,89 @@ struct scmi_powercap_proto_ops {
u32 *power_thresh_high);
};
+enum scmi_pinctrl_selector_type {
+ PIN_TYPE = 0,
+ GROUP_TYPE,
+ FUNCTION_TYPE,
+};
+
+enum scmi_pinctrl_conf_type {
+ SCMI_PIN_DEFAULT = 0,
+ SCMI_PIN_BIAS_BUS_HOLD = 1,
+ SCMI_PIN_BIAS_DISABLE = 2,
+ SCMI_PIN_BIAS_HIGH_IMPEDANCE = 3,
+ SCMI_PIN_BIAS_PULL_UP = 4,
+ SCMI_PIN_BIAS_PULL_DEFAULT = 5,
+ SCMI_PIN_BIAS_PULL_DOWN = 6,
+ SCMI_PIN_DRIVE_OPEN_DRAIN = 7,
+ SCMI_PIN_DRIVE_OPEN_SOURCE = 8,
+ SCMI_PIN_DRIVE_PUSH_PULL = 9,
+ SCMI_PIN_DRIVE_STRENGTH = 10,
+ SCMI_PIN_INPUT_DEBOUNCE = 11,
+ SCMI_PIN_INPUT_MODE = 12,
+ SCMI_PIN_PULL_MODE = 13,
+ SCMI_PIN_INPUT_VALUE = 14,
+ SCMI_PIN_INPUT_SCHMITT = 15,
+ SCMI_PIN_LOW_POWER_MODE = 16,
+ SCMI_PIN_OUTPUT_MODE = 17,
+ SCMI_PIN_OUTPUT_VALUE = 18,
+ SCMI_PIN_POWER_SOURCE = 19,
+ SCMI_PIN_SLEW_RATE = 20,
+ SCMI_PIN_OEM_START = 192,
+ SCMI_PIN_OEM_END = 255,
+};
+
+/**
+ * struct scmi_pinctrl_proto_ops - represents the various operations provided
+ * by SCMI Pinctrl Protocol
+ *
+ * @count_get: returns count of the registered elements in given type
+ * @name_get: returns name by index of given type
+ * @group_pins_get: returns the set of pins, assigned to the specified group
+ * @function_groups_get: returns the set of groups, assigned to the specified
+ * function
+ * @mux_set: set muxing function for groups of pins
+ * @settings_get_one: returns one configuration parameter for pin or group
+ * specified by config_type
+ * @settings_get_all: returns all configuration parameters for pin or group
+ * @settings_conf: sets the configuration parameter for pin or group
+ * @pin_request: aquire pin before selecting mux setting
+ * @pin_free: frees pin, acquired by request_pin call
+ */
+struct scmi_pinctrl_proto_ops {
+ int (*count_get)(const struct scmi_protocol_handle *ph,
+ enum scmi_pinctrl_selector_type type);
+ int (*name_get)(const struct scmi_protocol_handle *ph, u32 selector,
+ enum scmi_pinctrl_selector_type type,
+ const char **name);
+ int (*group_pins_get)(const struct scmi_protocol_handle *ph,
+ u32 selector, const unsigned int **pins,
+ unsigned int *nr_pins);
+ int (*function_groups_get)(const struct scmi_protocol_handle *ph,
+ u32 selector, unsigned int *nr_groups,
+ const unsigned int **groups);
+ int (*mux_set)(const struct scmi_protocol_handle *ph, u32 selector,
+ u32 group);
+ int (*settings_get_one)(const struct scmi_protocol_handle *ph,
+ u32 selector,
+ enum scmi_pinctrl_selector_type type,
+ enum scmi_pinctrl_conf_type config_type,
+ u32 *config_value);
+ int (*settings_get_all)(const struct scmi_protocol_handle *ph,
+ u32 selector,
+ enum scmi_pinctrl_selector_type type,
+ unsigned int *nr_configs,
+ enum scmi_pinctrl_conf_type *config_types,
+ u32 *config_values);
+ int (*settings_conf)(const struct scmi_protocol_handle *ph,
+ u32 selector, enum scmi_pinctrl_selector_type type,
+ unsigned int nr_configs,
+ enum scmi_pinctrl_conf_type *config_type,
+ u32 *config_value);
+ int (*pin_request)(const struct scmi_protocol_handle *ph, u32 pin);
+ int (*pin_free)(const struct scmi_protocol_handle *ph, u32 pin);
+};
+
/**
* struct scmi_notify_ops - represents notifications' operations provided by
* SCMI core
@@ -844,6 +927,7 @@ enum scmi_std_protocol {
SCMI_PROTOCOL_RESET = 0x16,
SCMI_PROTOCOL_VOLTAGE = 0x17,
SCMI_PROTOCOL_POWERCAP = 0x18,
+ SCMI_PROTOCOL_PINCTRL = 0x19,
};
enum scmi_system_events {
--
2.37.1
^ permalink raw reply related
* [PATCH v8 4/4] pinctrl: Implementation of the generic scmi-pinctrl driver
From: Peng Fan (OSS) @ 2024-04-05 1:59 UTC (permalink / raw)
To: Sudeep Holla, Cristian Marussi, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, Linus Walleij, Dan Carpenter
Cc: Andy Shevchenko, linux-arm-kernel, linux-kernel, devicetree,
linux-gpio, Peng Fan, Oleksii Moisieiev
In-Reply-To: <20240405-pinctrl-scmi-v8-0-5fc8e33871bf@nxp.com>
From: Peng Fan <peng.fan@nxp.com>
scmi-pinctrl driver implements pinctrl driver interface and using
SCMI protocol to redirect messages from pinctrl subsystem SDK to
SCMI platform firmware, which does the changes in HW.
Co-developed-by: Oleksii Moisieiev <oleksii_moisieiev@epam.com>
Signed-off-by: Oleksii Moisieiev <oleksii_moisieiev@epam.com>
Signed-off-by: Peng Fan <peng.fan@nxp.com>
---
MAINTAINERS | 1 +
drivers/pinctrl/Kconfig | 11 +
drivers/pinctrl/Makefile | 1 +
drivers/pinctrl/pinctrl-scmi.c | 564 +++++++++++++++++++++++++++++++++++++++++
4 files changed, 577 insertions(+)
diff --git a/MAINTAINERS b/MAINTAINERS
index 4b511a55101c..d8270ac6651a 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -21457,6 +21457,7 @@ F: drivers/cpufreq/sc[mp]i-cpufreq.c
F: drivers/firmware/arm_scmi/
F: drivers/firmware/arm_scpi.c
F: drivers/hwmon/scmi-hwmon.c
+F: drivers/pinctrl/pinctrl-scmi.c
F: drivers/pmdomain/arm/
F: drivers/powercap/arm_scmi_powercap.c
F: drivers/regulator/scmi-regulator.c
diff --git a/drivers/pinctrl/Kconfig b/drivers/pinctrl/Kconfig
index d45657aa986a..4e6f65cf0e76 100644
--- a/drivers/pinctrl/Kconfig
+++ b/drivers/pinctrl/Kconfig
@@ -450,6 +450,17 @@ config PINCTRL_ROCKCHIP
help
This support pinctrl and GPIO driver for Rockchip SoCs.
+config PINCTRL_SCMI
+ tristate "Pinctrl driver using SCMI protocol interface"
+ depends on ARM_SCMI_PROTOCOL || COMPILE_TEST
+ select PINMUX
+ select GENERIC_PINCONF
+ help
+ This driver provides support for pinctrl which is controlled
+ by firmware that implements the SCMI interface.
+ It uses SCMI Message Protocol to interact with the
+ firmware providing all the pinctrl controls.
+
config PINCTRL_SINGLE
tristate "One-register-per-pin type device tree based pinctrl driver"
depends on OF
diff --git a/drivers/pinctrl/Makefile b/drivers/pinctrl/Makefile
index 2152539b53d5..cc809669405a 100644
--- a/drivers/pinctrl/Makefile
+++ b/drivers/pinctrl/Makefile
@@ -45,6 +45,7 @@ obj-$(CONFIG_PINCTRL_PIC32) += pinctrl-pic32.o
obj-$(CONFIG_PINCTRL_PISTACHIO) += pinctrl-pistachio.o
obj-$(CONFIG_PINCTRL_RK805) += pinctrl-rk805.o
obj-$(CONFIG_PINCTRL_ROCKCHIP) += pinctrl-rockchip.o
+obj-$(CONFIG_PINCTRL_SCMI) += pinctrl-scmi.o
obj-$(CONFIG_PINCTRL_SINGLE) += pinctrl-single.o
obj-$(CONFIG_PINCTRL_ST) += pinctrl-st.o
obj-$(CONFIG_PINCTRL_STMFX) += pinctrl-stmfx.o
diff --git a/drivers/pinctrl/pinctrl-scmi.c b/drivers/pinctrl/pinctrl-scmi.c
new file mode 100644
index 000000000000..0f55f000a679
--- /dev/null
+++ b/drivers/pinctrl/pinctrl-scmi.c
@@ -0,0 +1,564 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * System Control and Power Interface (SCMI) Protocol based pinctrl driver
+ *
+ * Copyright (C) 2024 EPAM
+ * Copyright 2024 NXP
+ */
+
+#include <linux/device.h>
+#include <linux/dev_printk.h>
+#include <linux/err.h>
+#include <linux/module.h>
+#include <linux/scmi_protocol.h>
+#include <linux/slab.h>
+#include <linux/types.h>
+
+#include <linux/pinctrl/machine.h>
+#include <linux/pinctrl/pinconf.h>
+#include <linux/pinctrl/pinconf-generic.h>
+#include <linux/pinctrl/pinctrl.h>
+#include <linux/pinctrl/pinmux.h>
+
+#include "pinctrl-utils.h"
+#include "core.h"
+#include "pinconf.h"
+
+#define DRV_NAME "scmi-pinctrl"
+
+/* Define num configs, if not large than 4 use stack, else use kcalloc */
+#define SCMI_NUM_CONFIGS 4
+
+static const struct scmi_pinctrl_proto_ops *pinctrl_ops;
+
+struct scmi_pinctrl {
+ struct device *dev;
+ struct scmi_protocol_handle *ph;
+ struct pinctrl_dev *pctldev;
+ struct pinctrl_desc pctl_desc;
+ struct pinfunction *functions;
+ unsigned int nr_functions;
+ struct pinctrl_pin_desc *pins;
+ unsigned int nr_pins;
+};
+
+static int pinctrl_scmi_get_groups_count(struct pinctrl_dev *pctldev)
+{
+ struct scmi_pinctrl *pmx = pinctrl_dev_get_drvdata(pctldev);
+
+ return pinctrl_ops->count_get(pmx->ph, GROUP_TYPE);
+}
+
+static const char *pinctrl_scmi_get_group_name(struct pinctrl_dev *pctldev,
+ unsigned int selector)
+{
+ int ret;
+ const char *name;
+ struct scmi_pinctrl *pmx = pinctrl_dev_get_drvdata(pctldev);
+
+ ret = pinctrl_ops->name_get(pmx->ph, selector, GROUP_TYPE, &name);
+ if (ret) {
+ dev_err(pmx->dev, "get name failed with err %d", ret);
+ return NULL;
+ }
+
+ return name;
+}
+
+static int pinctrl_scmi_get_group_pins(struct pinctrl_dev *pctldev,
+ unsigned int selector,
+ const unsigned int **pins,
+ unsigned int *num_pins)
+{
+ struct scmi_pinctrl *pmx = pinctrl_dev_get_drvdata(pctldev);
+
+ return pinctrl_ops->group_pins_get(pmx->ph, selector, pins, num_pins);
+}
+
+static const struct pinctrl_ops pinctrl_scmi_pinctrl_ops = {
+ .get_groups_count = pinctrl_scmi_get_groups_count,
+ .get_group_name = pinctrl_scmi_get_group_name,
+ .get_group_pins = pinctrl_scmi_get_group_pins,
+#ifdef CONFIG_OF
+ .dt_node_to_map = pinconf_generic_dt_node_to_map_all,
+ .dt_free_map = pinconf_generic_dt_free_map,
+#endif
+};
+
+static int pinctrl_scmi_get_functions_count(struct pinctrl_dev *pctldev)
+{
+ struct scmi_pinctrl *pmx = pinctrl_dev_get_drvdata(pctldev);
+
+ return pinctrl_ops->count_get(pmx->ph, FUNCTION_TYPE);
+}
+
+static const char *pinctrl_scmi_get_function_name(struct pinctrl_dev *pctldev,
+ unsigned int selector)
+{
+ int ret;
+ const char *name;
+ struct scmi_pinctrl *pmx = pinctrl_dev_get_drvdata(pctldev);
+
+ ret = pinctrl_ops->name_get(pmx->ph, selector, FUNCTION_TYPE, &name);
+ if (ret) {
+ dev_err(pmx->dev, "get name failed with err %d", ret);
+ return NULL;
+ }
+
+ return name;
+}
+
+static int pinctrl_scmi_get_function_groups(struct pinctrl_dev *pctldev,
+ unsigned int selector,
+ const char * const **p_groups,
+ unsigned int * const p_num_groups)
+{
+ struct pinfunction *func;
+ const unsigned int *group_ids;
+ unsigned int num_groups;
+ const char **groups;
+ int ret, i;
+ struct scmi_pinctrl *pmx = pinctrl_dev_get_drvdata(pctldev);
+
+ if (!p_groups || !p_num_groups)
+ return -EINVAL;
+
+ if (selector >= pmx->nr_functions)
+ return -EINVAL;
+
+ func = &pmx->functions[selector];
+ if (func->ngroups)
+ goto done;
+
+ ret = pinctrl_ops->function_groups_get(pmx->ph, selector, &num_groups,
+ &group_ids);
+ if (ret) {
+ dev_err(pmx->dev, "Unable to get function groups, err %d", ret);
+ return ret;
+ }
+ if (!num_groups)
+ return -EINVAL;
+
+ groups = kcalloc(num_groups, sizeof(*groups), GFP_KERNEL);
+ if (!groups)
+ return -ENOMEM;
+
+ for (i = 0; i < num_groups; i++) {
+ groups[i] = pinctrl_scmi_get_group_name(pctldev, group_ids[i]);
+ if (!groups[i]) {
+ ret = -EINVAL;
+ goto err_free;
+ }
+ }
+
+ func->ngroups = num_groups;
+ func->groups = groups;
+done:
+ *p_groups = func->groups;
+ *p_num_groups = func->ngroups;
+
+ return 0;
+
+err_free:
+ kfree(groups);
+
+ return ret;
+}
+
+static int pinctrl_scmi_func_set_mux(struct pinctrl_dev *pctldev,
+ unsigned int selector, unsigned int group)
+{
+ struct scmi_pinctrl *pmx = pinctrl_dev_get_drvdata(pctldev);
+
+ return pinctrl_ops->mux_set(pmx->ph, selector, group);
+}
+
+static int pinctrl_scmi_request(struct pinctrl_dev *pctldev,
+ unsigned int offset)
+{
+ struct scmi_pinctrl *pmx = pinctrl_dev_get_drvdata(pctldev);
+
+ return pinctrl_ops->pin_request(pmx->ph, offset);
+}
+
+static int pinctrl_scmi_free(struct pinctrl_dev *pctldev, unsigned int offset)
+{
+ struct scmi_pinctrl *pmx = pinctrl_dev_get_drvdata(pctldev);
+
+ return pinctrl_ops->pin_free(pmx->ph, offset);
+}
+
+static const struct pinmux_ops pinctrl_scmi_pinmux_ops = {
+ .request = pinctrl_scmi_request,
+ .free = pinctrl_scmi_free,
+ .get_functions_count = pinctrl_scmi_get_functions_count,
+ .get_function_name = pinctrl_scmi_get_function_name,
+ .get_function_groups = pinctrl_scmi_get_function_groups,
+ .set_mux = pinctrl_scmi_func_set_mux,
+};
+
+static int pinctrl_scmi_map_pinconf_type(enum pin_config_param param,
+ enum scmi_pinctrl_conf_type *type)
+{
+ u32 arg = param;
+
+ switch (arg) {
+ case PIN_CONFIG_BIAS_BUS_HOLD:
+ *type = SCMI_PIN_BIAS_BUS_HOLD;
+ break;
+ case PIN_CONFIG_BIAS_DISABLE:
+ *type = SCMI_PIN_BIAS_DISABLE;
+ break;
+ case PIN_CONFIG_BIAS_HIGH_IMPEDANCE:
+ *type = SCMI_PIN_BIAS_HIGH_IMPEDANCE;
+ break;
+ case PIN_CONFIG_BIAS_PULL_DOWN:
+ *type = SCMI_PIN_BIAS_PULL_DOWN;
+ break;
+ case PIN_CONFIG_BIAS_PULL_PIN_DEFAULT:
+ *type = SCMI_PIN_BIAS_PULL_DEFAULT;
+ break;
+ case PIN_CONFIG_BIAS_PULL_UP:
+ *type = SCMI_PIN_BIAS_PULL_UP;
+ break;
+ case PIN_CONFIG_DRIVE_OPEN_DRAIN:
+ *type = SCMI_PIN_DRIVE_OPEN_DRAIN;
+ break;
+ case PIN_CONFIG_DRIVE_OPEN_SOURCE:
+ *type = SCMI_PIN_DRIVE_OPEN_SOURCE;
+ break;
+ case PIN_CONFIG_DRIVE_PUSH_PULL:
+ *type = SCMI_PIN_DRIVE_PUSH_PULL;
+ break;
+ case PIN_CONFIG_DRIVE_STRENGTH:
+ *type = SCMI_PIN_DRIVE_STRENGTH;
+ break;
+ case PIN_CONFIG_DRIVE_STRENGTH_UA:
+ *type = SCMI_PIN_DRIVE_STRENGTH;
+ break;
+ case PIN_CONFIG_INPUT_DEBOUNCE:
+ *type = SCMI_PIN_INPUT_DEBOUNCE;
+ break;
+ case PIN_CONFIG_INPUT_ENABLE:
+ *type = SCMI_PIN_INPUT_MODE;
+ break;
+ case PIN_CONFIG_INPUT_SCHMITT:
+ *type = SCMI_PIN_INPUT_SCHMITT;
+ break;
+ case PIN_CONFIG_INPUT_SCHMITT_ENABLE:
+ *type = SCMI_PIN_INPUT_MODE;
+ break;
+ case PIN_CONFIG_MODE_LOW_POWER:
+ *type = SCMI_PIN_LOW_POWER_MODE;
+ break;
+ case PIN_CONFIG_OUTPUT:
+ *type = SCMI_PIN_OUTPUT_VALUE;
+ break;
+ case PIN_CONFIG_OUTPUT_ENABLE:
+ *type = SCMI_PIN_OUTPUT_MODE;
+ break;
+ case PIN_CONFIG_OUTPUT_IMPEDANCE_OHMS:
+ *type = SCMI_PIN_OUTPUT_VALUE;
+ break;
+ case PIN_CONFIG_POWER_SOURCE:
+ *type = SCMI_PIN_POWER_SOURCE;
+ break;
+ case PIN_CONFIG_SLEW_RATE:
+ *type = SCMI_PIN_SLEW_RATE;
+ break;
+ case SCMI_PIN_OEM_START ... SCMI_PIN_OEM_END:
+ *type = arg;
+ break;
+ default:
+ return -EINVAL;
+ }
+
+ return 0;
+}
+
+static int pinctrl_scmi_pinconf_get(struct pinctrl_dev *pctldev,
+ unsigned int pin, unsigned long *config)
+{
+ int ret;
+ struct scmi_pinctrl *pmx = pinctrl_dev_get_drvdata(pctldev);
+ enum pin_config_param config_type;
+ enum scmi_pinctrl_conf_type type;
+ u32 config_value;
+
+ if (!config)
+ return -EINVAL;
+
+ config_type = pinconf_to_config_param(*config);
+
+ ret = pinctrl_scmi_map_pinconf_type(config_type, &type);
+ if (ret)
+ return ret;
+
+ ret = pinctrl_ops->settings_get_one(pmx->ph, pin, PIN_TYPE, type,
+ &config_value);
+ if (ret)
+ return ret;
+
+ *config = pinconf_to_config_packed(config_type, config_value);
+
+ return 0;
+}
+
+static int
+pinctrl_scmi_alloc_configs(struct pinctrl_dev *pctldev, u32 num_configs,
+ u32 **p_config_value,
+ enum scmi_pinctrl_conf_type **p_config_type)
+{
+ if (num_configs <= SCMI_NUM_CONFIGS)
+ return 0;
+
+ *p_config_value = kcalloc(num_configs, sizeof(**p_config_value), GFP_KERNEL);
+ if (!*p_config_value)
+ return -ENOMEM;
+
+ *p_config_type = kcalloc(num_configs, sizeof(**p_config_type), GFP_KERNEL);
+ if (!*p_config_type) {
+ kfree(*p_config_value);
+ return -ENOMEM;
+ }
+
+ return 0;
+}
+
+static void
+pinctrl_scmi_free_configs(struct pinctrl_dev *pctldev, u32 num_configs,
+ u32 **p_config_value,
+ enum scmi_pinctrl_conf_type **p_config_type)
+{
+ if (num_configs <= SCMI_NUM_CONFIGS)
+ return;
+
+ kfree(*p_config_value);
+ kfree(*p_config_type);
+}
+
+static int pinctrl_scmi_pinconf_set(struct pinctrl_dev *pctldev,
+ unsigned int pin,
+ unsigned long *configs,
+ unsigned int num_configs)
+{
+ int i, ret;
+ struct scmi_pinctrl *pmx = pinctrl_dev_get_drvdata(pctldev);
+ enum scmi_pinctrl_conf_type config_type[SCMI_NUM_CONFIGS];
+ u32 config_value[SCMI_NUM_CONFIGS];
+ enum scmi_pinctrl_conf_type *p_config_type = config_type;
+ u32 *p_config_value = config_value;
+ enum pin_config_param param;
+
+ if (!configs || !num_configs)
+ return -EINVAL;
+
+ ret = pinctrl_scmi_alloc_configs(pctldev, num_configs, &p_config_type,
+ &p_config_value);
+ if (ret)
+ return ret;
+
+ for (i = 0; i < num_configs; i++) {
+ param = pinconf_to_config_param(configs[i]);
+ ret = pinctrl_scmi_map_pinconf_type(param, &p_config_type[i]);
+ if (ret) {
+ dev_err(pmx->dev, "Error map pinconf_type %d\n", ret);
+ goto free_config;
+ }
+ p_config_value[i] = pinconf_to_config_argument(configs[i]);
+ }
+
+ ret = pinctrl_ops->settings_conf(pmx->ph, pin, PIN_TYPE, num_configs,
+ p_config_type, p_config_value);
+ if (ret)
+ dev_err(pmx->dev, "Error parsing config %d\n", ret);
+
+free_config:
+ pinctrl_scmi_free_configs(pctldev, num_configs, &p_config_type,
+ &p_config_value);
+ return ret;
+}
+
+static int pinctrl_scmi_pinconf_group_set(struct pinctrl_dev *pctldev,
+ unsigned int group,
+ unsigned long *configs,
+ unsigned int num_configs)
+{
+ int i, ret;
+ struct scmi_pinctrl *pmx = pinctrl_dev_get_drvdata(pctldev);
+ enum scmi_pinctrl_conf_type config_type[SCMI_NUM_CONFIGS];
+ u32 config_value[SCMI_NUM_CONFIGS];
+ enum scmi_pinctrl_conf_type *p_config_type = config_type;
+ u32 *p_config_value = config_value;
+ enum pin_config_param param;
+
+ if (!configs || !num_configs)
+ return -EINVAL;
+
+ ret = pinctrl_scmi_alloc_configs(pctldev, num_configs, &p_config_type,
+ &p_config_value);
+ if (ret)
+ return ret;
+
+ for (i = 0; i < num_configs; i++) {
+ param = pinconf_to_config_param(configs[i]);
+ ret = pinctrl_scmi_map_pinconf_type(param, &p_config_type[i]);
+ if (ret) {
+ dev_err(pmx->dev, "Error map pinconf_type %d\n", ret);
+ goto free_config;
+ }
+
+ p_config_value[i] = pinconf_to_config_argument(configs[i]);
+ }
+
+ ret = pinctrl_ops->settings_conf(pmx->ph, group, GROUP_TYPE,
+ num_configs, p_config_type,
+ p_config_value);
+ if (ret)
+ dev_err(pmx->dev, "Error parsing config %d", ret);
+
+free_config:
+ pinctrl_scmi_free_configs(pctldev, num_configs, &p_config_type,
+ &p_config_value);
+ return ret;
+};
+
+static int pinctrl_scmi_pinconf_group_get(struct pinctrl_dev *pctldev,
+ unsigned int group,
+ unsigned long *config)
+{
+ int ret;
+ struct scmi_pinctrl *pmx = pinctrl_dev_get_drvdata(pctldev);
+ enum pin_config_param config_type;
+ enum scmi_pinctrl_conf_type type;
+ u32 config_value;
+
+ if (!config)
+ return -EINVAL;
+
+ config_type = pinconf_to_config_param(*config);
+ ret = pinctrl_scmi_map_pinconf_type(config_type, &type);
+ if (ret) {
+ dev_err(pmx->dev, "Error map pinconf_type %d\n", ret);
+ return ret;
+ }
+
+ ret = pinctrl_ops->settings_get_one(pmx->ph, group, GROUP_TYPE, type,
+ &config_value);
+ if (ret)
+ return ret;
+
+ *config = pinconf_to_config_packed(config_type, config_value);
+
+ return 0;
+}
+
+static const struct pinconf_ops pinctrl_scmi_pinconf_ops = {
+ .is_generic = true,
+ .pin_config_get = pinctrl_scmi_pinconf_get,
+ .pin_config_set = pinctrl_scmi_pinconf_set,
+ .pin_config_group_set = pinctrl_scmi_pinconf_group_set,
+ .pin_config_group_get = pinctrl_scmi_pinconf_group_get,
+ .pin_config_config_dbg_show = pinconf_generic_dump_config,
+};
+
+static int pinctrl_scmi_get_pins(struct scmi_pinctrl *pmx,
+ struct pinctrl_desc *desc)
+{
+ struct pinctrl_pin_desc *pins;
+ unsigned int npins;
+ int ret, i;
+
+ npins = pinctrl_ops->count_get(pmx->ph, PIN_TYPE);
+ /*
+ * npins will never be zero, the scmi pinctrl driver has bailed out
+ * if npins is zero.
+ */
+ pins = devm_kmalloc_array(pmx->dev, npins, sizeof(*pins), GFP_KERNEL);
+ if (!pins)
+ return -ENOMEM;
+
+ for (i = 0; i < npins; i++) {
+ pins[i].number = i;
+ /*
+ * The memory for name is handled by the scmi firmware driver,
+ * no need free here
+ */
+ ret = pinctrl_ops->name_get(pmx->ph, i, PIN_TYPE, &pins[i].name);
+ if (ret)
+ return dev_err_probe(pmx->dev, ret,
+ "Can't get name for pin %d", i);
+ }
+
+ desc->npins = npins;
+ desc->pins = pins;
+ dev_dbg(pmx->dev, "got pins %u", npins);
+
+ return 0;
+}
+
+static int scmi_pinctrl_probe(struct scmi_device *sdev)
+{
+ int ret;
+ struct device *dev = &sdev->dev;
+ struct scmi_pinctrl *pmx;
+ const struct scmi_handle *handle;
+ struct scmi_protocol_handle *ph;
+
+ if (!sdev->handle)
+ return -EINVAL;
+
+ handle = sdev->handle;
+
+ pinctrl_ops = handle->devm_protocol_get(sdev, SCMI_PROTOCOL_PINCTRL, &ph);
+ if (IS_ERR(pinctrl_ops))
+ return PTR_ERR(pinctrl_ops);
+
+ pmx = devm_kzalloc(dev, sizeof(*pmx), GFP_KERNEL);
+ if (!pmx)
+ return -ENOMEM;
+
+ pmx->ph = ph;
+
+ pmx->dev = dev;
+ pmx->pctl_desc.name = DRV_NAME;
+ pmx->pctl_desc.owner = THIS_MODULE;
+ pmx->pctl_desc.pctlops = &pinctrl_scmi_pinctrl_ops;
+ pmx->pctl_desc.pmxops = &pinctrl_scmi_pinmux_ops;
+ pmx->pctl_desc.confops = &pinctrl_scmi_pinconf_ops;
+
+ ret = pinctrl_scmi_get_pins(pmx, &pmx->pctl_desc);
+ if (ret)
+ return ret;
+
+ ret = devm_pinctrl_register_and_init(dev, &pmx->pctl_desc, pmx,
+ &pmx->pctldev);
+ if (ret)
+ return dev_err_probe(dev, ret, "Failed to register pinctrl\n");
+
+ pmx->nr_functions = pinctrl_scmi_get_functions_count(pmx->pctldev);
+ pmx->functions = devm_kcalloc(dev, pmx->nr_functions,
+ sizeof(*pmx->functions), GFP_KERNEL);
+ if (!pmx->functions)
+ return -ENOMEM;
+
+ return pinctrl_enable(pmx->pctldev);
+}
+
+static const struct scmi_device_id scmi_id_table[] = {
+ { SCMI_PROTOCOL_PINCTRL, "pinctrl" },
+ { }
+};
+MODULE_DEVICE_TABLE(scmi, scmi_id_table);
+
+static struct scmi_driver scmi_pinctrl_driver = {
+ .name = DRV_NAME,
+ .probe = scmi_pinctrl_probe,
+ .id_table = scmi_id_table,
+};
+module_scmi_driver(scmi_pinctrl_driver);
+
+MODULE_AUTHOR("Oleksii Moisieiev <oleksii_moisieiev@epam.com>");
+MODULE_AUTHOR("Peng Fan <peng.fan@nxp.com>");
+MODULE_DESCRIPTION("ARM SCMI pin controller driver");
+MODULE_LICENSE("GPL");
--
2.37.1
^ permalink raw reply related
* Re: [PATCH v6 0/1] Add StarFive JH8100 dwmac support
From: patchwork-bot+netdevbpf @ 2024-04-05 2:40 UTC (permalink / raw)
To: ChunHau Tan
Cc: davem, edumazet, kuba, pabeni, robh+dt, kernel, robh,
krzysztof.kozlowski+dt, conor+dt, mcoquelin.stm32,
alexandre.torgue, horms, bartosz.golaszewski, ahalaney, jszhang,
u.kleine-koenig, rmk+kernel, leyfoon.tan, jeeheng.sia, netdev,
devicetree, linux-kernel, linux-stm32, linux-arm-kernel,
linux-riscv
In-Reply-To: <20240403100549.78719-1-chunhau.tan@starfivetech.com>
Hello:
This patch was applied to netdev/net-next.git (main)
by Jakub Kicinski <kuba@kernel.org>:
On Wed, 3 Apr 2024 03:05:48 -0700 you wrote:
> Add StarFive JH8100 dwmac support.
> The JH8100 dwmac shares the same driver code as the JH7110 dwmac
> and has only one reset signal.
>
> Please refer to below:
>
> JH8100: reset-names = "stmmaceth";
> JH7110: reset-names = "stmmaceth", "ahb";
> JH7100: reset-names = "ahb";
>
> [...]
Here is the summary with links:
- [v6,1/1] dt-bindings: net: starfive,jh7110-dwmac: Add StarFive JH8100 support
https://git.kernel.org/netdev/net-next/c/1a9de5646559
You are awesome, thank you!
--
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html
^ permalink raw reply
* Re: [PATCH v2 2/2] arm64: dts: qcom: qcs6490-rb3gen2: Enable various remoteprocs
From: Bjorn Andersson @ 2024-04-05 2:53 UTC (permalink / raw)
To: Komal Bajaj
Cc: Bjorn Andersson, Konrad Dybcio, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, linux-arm-msm, devicetree, linux-kernel, quic_tsoni
In-Reply-To: <20240402090349.30172-3-quic_kbajaj@quicinc.com>
On Tue, Apr 02, 2024 at 02:33:49PM +0530, Komal Bajaj wrote:
> Enable the ADSP, CDSP and WPSS that are found on qcs6490-rb3gen2.
>
> Signed-off-by: Komal Bajaj <quic_kbajaj@quicinc.com>
> ---
> arch/arm64/boot/dts/qcom/qcs6490-rb3gen2.dts | 15 +++++++++++++++
> 1 file changed, 15 insertions(+)
>
> diff --git a/arch/arm64/boot/dts/qcom/qcs6490-rb3gen2.dts b/arch/arm64/boot/dts/qcom/qcs6490-rb3gen2.dts
> index 97824c769ba3..a25431ddf922 100644
> --- a/arch/arm64/boot/dts/qcom/qcs6490-rb3gen2.dts
> +++ b/arch/arm64/boot/dts/qcom/qcs6490-rb3gen2.dts
> @@ -434,6 +434,21 @@ &qupv3_id_0 {
> status = "okay";
> };
>
> +&remoteproc_adsp {
> + firmware-name = "qcom/qcm6490/adsp.mbn";
Should this be qcm6490?
I already proposed a patch to add adsp and cdsp, using qcs6490, and this
was merged earlier this week. Please pay attention and review patches
posted on the public list.
Either way, this will now have to be rebased on linux-next.
Thanks,
Bjorn
> + status = "okay";
> +};
> +
> +&remoteproc_cdsp {
> + firmware-name = "qcom/qcm6490/cdsp.mbn";
> + status = "okay";
> +};
> +
> +&remoteproc_wpss {
> + firmware-name = "qcom/qcm6490/wpss.mbn";
> + status = "okay";
> +};
> +
> &tlmm {
> gpio-reserved-ranges = <32 2>, /* ADSP */
> <48 4>; /* NFC */
> --
> 2.42.0
>
^ permalink raw reply
* Re: [PATCH v17 11/35] virt: gunyah: Translate gh_rm_hyp_resource into gunyah_resource
From: Pavan Kondeti @ 2024-04-05 3:10 UTC (permalink / raw)
To: Elliot Berman
Cc: Alex Elder, Srinivas Kandagatla, Murali Nalajal, Trilok Soni,
Srivatsa Vaddagiri, Carl van Schaik, Philip Derrin,
Prakruthi Deepak Heragu, Jonathan Corbet, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Catalin Marinas, Will Deacon,
Konrad Dybcio, Bjorn Andersson, Dmitry Baryshkov, Fuad Tabba,
Sean Christopherson, Andrew Morton, linux-arm-msm, linux-doc,
linux-kernel, devicetree, linux-arm-kernel, linux-mm
In-Reply-To: <20240222-gunyah-v17-11-1e9da6763d38@quicinc.com>
On Thu, Feb 22, 2024 at 03:16:34PM -0800, Elliot Berman wrote:
> When booting a Gunyah virtual machine, the host VM may gain capabilities
> to interact with resources for the guest virtual machine. Examples of
> such resources are vCPUs or message queues. To use those resources, we
> need to translate the RM response into a gunyah_resource structure which
> are useful to Linux drivers. Presently, Linux drivers need only to know
> the type of resource, the capability ID, and an interrupt.
>
> On ARM64 systems, the interrupt reported by Gunyah is the GIC interrupt
> ID number and always a SPI or extended SPI.
>
> Signed-off-by: Elliot Berman <quic_eberman@quicinc.com>
> ---
> arch/arm64/include/asm/gunyah.h | 36 ++++++++++++++++++++++
> drivers/virt/gunyah/rsc_mgr.c | 67 +++++++++++++++++++++++++++++++++++++++++
> drivers/virt/gunyah/rsc_mgr.h | 5 +++
> include/linux/gunyah.h | 2 ++
> 4 files changed, 110 insertions(+)
>
> diff --git a/arch/arm64/include/asm/gunyah.h b/arch/arm64/include/asm/gunyah.h
> new file mode 100644
> index 0000000000000..0cd3debe22b64
> --- /dev/null
> +++ b/arch/arm64/include/asm/gunyah.h
> @@ -0,0 +1,36 @@
> +/* SPDX-License-Identifier: GPL-2.0-only */
> +/*
> + * Copyright (c) 2022-2023 Qualcomm Innovation Center, Inc. All rights reserved.
> + */
> +#ifndef _ASM_GUNYAH_H
> +#define _ASM_GUNYAH_H
> +
> +#include <linux/irq.h>
> +#include <linux/irqdomain.h>
> +
> +static inline int arch_gunyah_fill_irq_fwspec_params(u32 virq,
> + struct irq_fwspec *fwspec)
> +{
> + /* Assume that Gunyah gave us an SPI or ESPI; defensively check it */
> + if (WARN(virq < 32, "Unexpected virq: %d\n", virq)) {
> + return -EINVAL;
> + } else if (virq <= 1019) {
> + fwspec->param_count = 3;
> + fwspec->param[0] = 0; /* GIC_SPI */
> + fwspec->param[1] = virq - 32; /* virq 32 -> SPI 0 */
> + fwspec->param[2] = IRQ_TYPE_EDGE_RISING;
> + } else if (WARN(virq < 4096, "Unexpected virq: %d\n", virq)) {
> + return -EINVAL;
> + } else if (virq < 5120) {
> + fwspec->param_count = 3;
> + fwspec->param[0] = 2; /* GIC_ESPI */
> + fwspec->param[1] = virq - 4096; /* virq 4096 -> ESPI 0 */
> + fwspec->param[2] = IRQ_TYPE_EDGE_RISING;
> + } else {
> + WARN(1, "Unexpected virq: %d\n", virq);
> + return -EINVAL;
> + }
> + return 0;
> +}
> +
__get_intid_range() in gic-v3 driver looks more pleasing. Other than
that the logic for the translation looks good to me.
Thanks,
Pavan
^ permalink raw reply
* [PATCH v5 0/4] LVDS Controller Support for SAM9X75 SoC
From: Dharma Balasubiramani @ 2024-04-05 4:35 UTC (permalink / raw)
To: andrzej . hajda @ intel . com, neil . armstrong @ linaro . org,
rfoss @ kernel . org, Laurent . pinchart @ ideasonboard . com,
jonas @ kwiboo . se, jernej . skrabec @ gmail . com,
maarten . lankhorst @ linux . intel . com, mripard @ kernel . org,
tzimmermann @ suse . de, airlied @ gmail . com,
daniel @ ffwll . ch, robh+dt @ kernel . org,
krzysztof . kozlowski+dt @ linaro . org, conor+dt @ kernel . org,
linux @ armlinux . org . uk, Nicolas . Ferre @ microchip . com,
alexandre . belloni @ bootlin . com,
claudiu . beznea @ tuxon . dev, Manikandan . M @ microchip . com,
Dharma . B @ microchip . com, arnd @ arndb . de,
geert+renesas @ glider . be, Jason @ zx2c4 . com,
mpe @ ellerman . id . au, gerg @ linux-m68k . org,
rdunlap @ infradead . org, vbabka @ suse . cz,
dri-devel @ lists . freedesktop . org,
devicetree @ vger . kernel . org,
linux-kernel @ vger . kernel . org,
linux-arm-kernel @ lists . infradead . org,
Hari . PrasathGE @ microchip . com, akpm @ linux-foundation . org,
deller @ gmx . de
Cc: Dharma Balasubiramani
This patch series introduces LVDS controller support for the SAM9X75 SoC. The
LVDS controller is designed to work with Microchip's sam9x7 series
System-on-Chip (SoC) devices, providing Low Voltage Differential Signaling
capabilities.
Patch series Changelog:
- Include configs: at91: Enable LVDS serializer
- include all necessary To/Cc entries.
The Individual Changelogs are available on the respective patches.
Dharma Balasubiramani (4):
dt-bindings: display: bridge: add sam9x75-lvds binding
drm/bridge: add lvds controller support for sam9x7
MAINTAINERS: add SAM9X7 SoC's LVDS controller
ARM: configs: at91: Enable LVDS serializer support
.../bridge/microchip,sam9x75-lvds.yaml | 55 +++++
MAINTAINERS | 8 +
arch/arm/configs/at91_dt_defconfig | 1 +
drivers/gpu/drm/bridge/Kconfig | 7 +
drivers/gpu/drm/bridge/Makefile | 1 +
drivers/gpu/drm/bridge/microchip-lvds.c | 228 ++++++++++++++++++
6 files changed, 300 insertions(+)
create mode 100644 Documentation/devicetree/bindings/display/bridge/microchip,sam9x75-lvds.yaml
create mode 100644 drivers/gpu/drm/bridge/microchip-lvds.c
--
2.25.1
^ permalink raw reply
* [PATCH v5 1/4] dt-bindings: display: bridge: add sam9x75-lvds binding
From: Dharma Balasubiramani @ 2024-04-05 4:35 UTC (permalink / raw)
To: andrzej . hajda @ intel . com, neil . armstrong @ linaro . org,
rfoss @ kernel . org, Laurent . pinchart @ ideasonboard . com,
jonas @ kwiboo . se, jernej . skrabec @ gmail . com,
maarten . lankhorst @ linux . intel . com, mripard @ kernel . org,
tzimmermann @ suse . de, airlied @ gmail . com,
daniel @ ffwll . ch, robh+dt @ kernel . org,
krzysztof . kozlowski+dt @ linaro . org, conor+dt @ kernel . org,
linux @ armlinux . org . uk, Nicolas . Ferre @ microchip . com,
alexandre . belloni @ bootlin . com,
claudiu . beznea @ tuxon . dev, Manikandan . M @ microchip . com,
Dharma . B @ microchip . com, arnd @ arndb . de,
geert+renesas @ glider . be, Jason @ zx2c4 . com,
mpe @ ellerman . id . au, gerg @ linux-m68k . org,
rdunlap @ infradead . org, vbabka @ suse . cz,
dri-devel @ lists . freedesktop . org,
devicetree @ vger . kernel . org,
linux-kernel @ vger . kernel . org,
linux-arm-kernel @ lists . infradead . org,
Hari . PrasathGE @ microchip . com, akpm @ linux-foundation . org,
deller @ gmx . de
Cc: Dharma Balasubiramani, Rob Herring
In-Reply-To: <20240405043536.274220-1-dharma.b@microchip.com>
Add the 'sam9x75-lvds' compatible binding, which describes the Low Voltage
Differential Signaling (LVDS) Controller found on some Microchip's sam9x7
series System-on-Chip (SoC) devices. This binding will be used to define
the properties and configuration for the LVDS Controller in DT.
Signed-off-by: Dharma Balasubiramani <dharma.b@microchip.com>
Reviewed-by: Rob Herring <robh@kernel.org>
---
Changelog
v4 -> v5
- No changes.
v3 -> v4
- Rephrase the commit subject.
v2 -> v3
- No changes.
v1 -> v2
- Remove '|' in description, as there is no formatting to preserve.
- Remove 'gclk' from clock-names as there is only one clock(pclk).
- Remove the unused headers and include only used ones.
- Change the compatible name specific to SoC (sam9x75) instead of entire series.
- Change file name to match the compatible name.
---
.../bridge/microchip,sam9x75-lvds.yaml | 55 +++++++++++++++++++
1 file changed, 55 insertions(+)
create mode 100644 Documentation/devicetree/bindings/display/bridge/microchip,sam9x75-lvds.yaml
diff --git a/Documentation/devicetree/bindings/display/bridge/microchip,sam9x75-lvds.yaml b/Documentation/devicetree/bindings/display/bridge/microchip,sam9x75-lvds.yaml
new file mode 100644
index 000000000000..862ef441ac9f
--- /dev/null
+++ b/Documentation/devicetree/bindings/display/bridge/microchip,sam9x75-lvds.yaml
@@ -0,0 +1,55 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/display/bridge/microchip,sam9x75-lvds.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Microchip SAM9X75 LVDS Controller
+
+maintainers:
+ - Dharma Balasubiramani <dharma.b@microchip.com>
+
+description:
+ The Low Voltage Differential Signaling Controller (LVDSC) manages data
+ format conversion from the LCD Controller internal DPI bus to OpenLDI
+ LVDS output signals. LVDSC functions include bit mapping, balanced mode
+ management, and serializer.
+
+properties:
+ compatible:
+ const: microchip,sam9x75-lvds
+
+ reg:
+ maxItems: 1
+
+ interrupts:
+ maxItems: 1
+
+ clocks:
+ items:
+ - description: Peripheral Bus Clock
+
+ clock-names:
+ items:
+ - const: pclk
+
+required:
+ - compatible
+ - reg
+ - interrupts
+ - clocks
+ - clock-names
+
+additionalProperties: false
+
+examples:
+ - |
+ #include <dt-bindings/interrupt-controller/irq.h>
+ #include <dt-bindings/clock/at91.h>
+ lvds-controller@f8060000 {
+ compatible = "microchip,sam9x75-lvds";
+ reg = <0xf8060000 0x100>;
+ interrupts = <56 IRQ_TYPE_LEVEL_HIGH 0>;
+ clocks = <&pmc PMC_TYPE_PERIPHERAL 56>;
+ clock-names = "pclk";
+ };
--
2.25.1
^ permalink raw reply related
* [PATCH v5 2/4] drm/bridge: add lvds controller support for sam9x7
From: Dharma Balasubiramani @ 2024-04-05 4:35 UTC (permalink / raw)
To: andrzej . hajda @ intel . com, neil . armstrong @ linaro . org,
rfoss @ kernel . org, Laurent . pinchart @ ideasonboard . com,
jonas @ kwiboo . se, jernej . skrabec @ gmail . com,
maarten . lankhorst @ linux . intel . com, mripard @ kernel . org,
tzimmermann @ suse . de, airlied @ gmail . com,
daniel @ ffwll . ch, robh+dt @ kernel . org,
krzysztof . kozlowski+dt @ linaro . org, conor+dt @ kernel . org,
linux @ armlinux . org . uk, Nicolas . Ferre @ microchip . com,
alexandre . belloni @ bootlin . com,
claudiu . beznea @ tuxon . dev, Manikandan . M @ microchip . com,
Dharma . B @ microchip . com, arnd @ arndb . de,
geert+renesas @ glider . be, Jason @ zx2c4 . com,
mpe @ ellerman . id . au, gerg @ linux-m68k . org,
rdunlap @ infradead . org, vbabka @ suse . cz,
dri-devel @ lists . freedesktop . org,
devicetree @ vger . kernel . org,
linux-kernel @ vger . kernel . org,
linux-arm-kernel @ lists . infradead . org,
Hari . PrasathGE @ microchip . com, akpm @ linux-foundation . org,
deller @ gmx . de
Cc: Dharma Balasubiramani, Manikandan Muralidharan
In-Reply-To: <20240405043536.274220-1-dharma.b@microchip.com>
Add a new LVDS controller driver for sam9x7 which does the following:
- Prepares and enables the LVDS Peripheral clock
- Defines its connector type as DRM_MODE_CONNECTOR_LVDS and adds itself
to the global bridge list.
- Identifies its output endpoint as panel and adds it to the encoder
display pipeline
- Enables the LVDS serializer
Signed-off-by: Manikandan Muralidharan <manikandan.m@microchip.com>
Signed-off-by: Dharma Balasubiramani <dharma.b@microchip.com>
---
Changelog
v4 -> v5
- Drop the unused variable 'format'.
- Use DRM wrapper for dev_err() to maintain uniformity.
- return -ENODEV instead of -EINVAL to maintain consistency with other DRM
bridge drivers.
v3 -> v4
- No changes.
v2 ->v3
- Correct Typo error "serializer".
- Consolidate get() and prepare() functions and use devm_clk_get_prepared().
- Remove unused variable 'ret' in probe().
- Use devm_pm_runtime_enable() and drop the mchp_lvds_remove().
v1 -> v2
- Drop 'res' variable and combine two lines into one.
- Handle deferred probe properly, use dev_err_probe().
- Don't print anything on deferred probe. Dropped print.
- Remove the MODULE_ALIAS and add MODULE_DEVICE_TABLE().
- symbol 'mchp_lvds_driver' was not declared. It should be static.
---
drivers/gpu/drm/bridge/Kconfig | 7 +
drivers/gpu/drm/bridge/Makefile | 1 +
drivers/gpu/drm/bridge/microchip-lvds.c | 228 ++++++++++++++++++++++++
3 files changed, 236 insertions(+)
create mode 100644 drivers/gpu/drm/bridge/microchip-lvds.c
diff --git a/drivers/gpu/drm/bridge/Kconfig b/drivers/gpu/drm/bridge/Kconfig
index efd996f6c138..889098e2d65f 100644
--- a/drivers/gpu/drm/bridge/Kconfig
+++ b/drivers/gpu/drm/bridge/Kconfig
@@ -190,6 +190,13 @@ config DRM_MEGACHIPS_STDPXXXX_GE_B850V3_FW
to DP++. This is used with the i.MX6 imx-ldb
driver. You are likely to say N here.
+config DRM_MICROCHIP_LVDS_SERIALIZER
+ tristate "Microchip LVDS serializer support"
+ depends on OF
+ depends on DRM_ATMEL_HLCDC
+ help
+ Support for Microchip's LVDS serializer.
+
config DRM_NWL_MIPI_DSI
tristate "Northwest Logic MIPI DSI Host controller"
depends on DRM
diff --git a/drivers/gpu/drm/bridge/Makefile b/drivers/gpu/drm/bridge/Makefile
index 017b5832733b..7df87b582dca 100644
--- a/drivers/gpu/drm/bridge/Makefile
+++ b/drivers/gpu/drm/bridge/Makefile
@@ -13,6 +13,7 @@ obj-$(CONFIG_DRM_LONTIUM_LT9611) += lontium-lt9611.o
obj-$(CONFIG_DRM_LONTIUM_LT9611UXC) += lontium-lt9611uxc.o
obj-$(CONFIG_DRM_LVDS_CODEC) += lvds-codec.o
obj-$(CONFIG_DRM_MEGACHIPS_STDPXXXX_GE_B850V3_FW) += megachips-stdpxxxx-ge-b850v3-fw.o
+obj-$(CONFIG_DRM_MICROCHIP_LVDS_SERIALIZER) += microchip-lvds.o
obj-$(CONFIG_DRM_NXP_PTN3460) += nxp-ptn3460.o
obj-$(CONFIG_DRM_PARADE_PS8622) += parade-ps8622.o
obj-$(CONFIG_DRM_PARADE_PS8640) += parade-ps8640.o
diff --git a/drivers/gpu/drm/bridge/microchip-lvds.c b/drivers/gpu/drm/bridge/microchip-lvds.c
new file mode 100644
index 000000000000..149704f498a6
--- /dev/null
+++ b/drivers/gpu/drm/bridge/microchip-lvds.c
@@ -0,0 +1,228 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * Copyright (C) 2023 Microchip Technology Inc. and its subsidiaries
+ *
+ * Author: Manikandan Muralidharan <manikandan.m@microchip.com>
+ * Author: Dharma Balasubiramani <dharma.b@microchip.com>
+ *
+ */
+
+#include <linux/clk.h>
+#include <linux/component.h>
+#include <linux/delay.h>
+#include <linux/jiffies.h>
+#include <linux/mfd/syscon.h>
+#include <linux/of_graph.h>
+#include <linux/pinctrl/devinfo.h>
+#include <linux/phy/phy.h>
+#include <linux/platform_device.h>
+#include <linux/pm_runtime.h>
+#include <linux/regmap.h>
+#include <linux/reset.h>
+
+#include <drm/drm_atomic_helper.h>
+#include <drm/drm_bridge.h>
+#include <drm/drm_of.h>
+#include <drm/drm_panel.h>
+#include <drm/drm_print.h>
+#include <drm/drm_probe_helper.h>
+#include <drm/drm_simple_kms_helper.h>
+
+#define LVDS_POLL_TIMEOUT_MS 1000
+
+/* LVDSC register offsets */
+#define LVDSC_CR 0x00
+#define LVDSC_CFGR 0x04
+#define LVDSC_SR 0x0C
+#define LVDSC_WPMR 0xE4
+
+/* Bitfields in LVDSC_CR (Control Register) */
+#define LVDSC_CR_SER_EN BIT(0)
+
+/* Bitfields in LVDSC_CFGR (Configuration Register) */
+#define LVDSC_CFGR_PIXSIZE_24BITS 0
+#define LVDSC_CFGR_DEN_POL_HIGH 0
+#define LVDSC_CFGR_DC_UNBALANCED 0
+#define LVDSC_CFGR_MAPPING_JEIDA BIT(6)
+
+/*Bitfields in LVDSC_SR */
+#define LVDSC_SR_CS BIT(0)
+
+/* Bitfields in LVDSC_WPMR (Write Protection Mode Register) */
+#define LVDSC_WPMR_WPKEY_MASK GENMASK(31, 8)
+#define LVDSC_WPMR_WPKEY_PSSWD 0x4C5644
+
+struct mchp_lvds {
+ struct device *dev;
+ void __iomem *regs;
+ struct clk *pclk;
+ struct drm_panel *panel;
+ struct drm_bridge bridge;
+ struct drm_bridge *panel_bridge;
+};
+
+static inline struct mchp_lvds *bridge_to_lvds(struct drm_bridge *bridge)
+{
+ return container_of(bridge, struct mchp_lvds, bridge);
+}
+
+static inline u32 lvds_readl(struct mchp_lvds *lvds, u32 offset)
+{
+ return readl_relaxed(lvds->regs + offset);
+}
+
+static inline void lvds_writel(struct mchp_lvds *lvds, u32 offset, u32 val)
+{
+ writel_relaxed(val, lvds->regs + offset);
+}
+
+static void lvds_serialiser_on(struct mchp_lvds *lvds)
+{
+ unsigned long timeout = jiffies + msecs_to_jiffies(LVDS_POLL_TIMEOUT_MS);
+
+ /* The LVDSC registers can only be written if WPEN is cleared */
+ lvds_writel(lvds, LVDSC_WPMR, (LVDSC_WPMR_WPKEY_PSSWD &
+ LVDSC_WPMR_WPKEY_MASK));
+
+ /* Wait for the status of configuration registers to be changed */
+ while (lvds_readl(lvds, LVDSC_SR) & LVDSC_SR_CS) {
+ if (time_after(jiffies, timeout)) {
+ DRM_DEV_ERROR(lvds->dev, "%s: timeout error\n",
+ __func__);
+ return;
+ }
+ usleep_range(1000, 2000);
+ }
+
+ /* Configure the LVDSC */
+ lvds_writel(lvds, LVDSC_CFGR, (LVDSC_CFGR_MAPPING_JEIDA |
+ LVDSC_CFGR_DC_UNBALANCED |
+ LVDSC_CFGR_DEN_POL_HIGH |
+ LVDSC_CFGR_PIXSIZE_24BITS));
+
+ /* Enable the LVDS serializer */
+ lvds_writel(lvds, LVDSC_CR, LVDSC_CR_SER_EN);
+}
+
+static int mchp_lvds_attach(struct drm_bridge *bridge,
+ enum drm_bridge_attach_flags flags)
+{
+ struct mchp_lvds *lvds = bridge_to_lvds(bridge);
+
+ bridge->encoder->encoder_type = DRM_MODE_ENCODER_LVDS;
+
+ return drm_bridge_attach(bridge->encoder, lvds->panel_bridge,
+ bridge, flags);
+}
+
+static void mchp_lvds_enable(struct drm_bridge *bridge)
+{
+ struct mchp_lvds *lvds = bridge_to_lvds(bridge);
+ int ret;
+
+ ret = clk_enable(lvds->pclk);
+ if (ret < 0) {
+ DRM_DEV_ERROR(lvds->dev, "failed to enable lvds pclk %d\n", ret);
+ return;
+ }
+
+ ret = pm_runtime_get_sync(lvds->dev);
+ if (ret < 0) {
+ DRM_DEV_ERROR(lvds->dev, "failed to get pm runtime: %d\n", ret);
+ clk_disable(lvds->pclk);
+ return;
+ }
+
+ lvds_serialiser_on(lvds);
+}
+
+static void mchp_lvds_disable(struct drm_bridge *bridge)
+{
+ struct mchp_lvds *lvds = bridge_to_lvds(bridge);
+
+ pm_runtime_put(lvds->dev);
+ clk_disable(lvds->pclk);
+}
+
+static const struct drm_bridge_funcs mchp_lvds_bridge_funcs = {
+ .attach = mchp_lvds_attach,
+ .enable = mchp_lvds_enable,
+ .disable = mchp_lvds_disable,
+};
+
+static int mchp_lvds_probe(struct platform_device *pdev)
+{
+ struct device *dev = &pdev->dev;
+ struct mchp_lvds *lvds;
+ struct device_node *port;
+
+ if (!dev->of_node)
+ return -ENODEV;
+
+ lvds = devm_kzalloc(&pdev->dev, sizeof(*lvds), GFP_KERNEL);
+ if (!lvds)
+ return -ENOMEM;
+
+ lvds->dev = dev;
+
+ lvds->regs = devm_ioremap_resource(lvds->dev,
+ platform_get_resource(pdev, IORESOURCE_MEM, 0));
+ if (IS_ERR(lvds->regs))
+ return PTR_ERR(lvds->regs);
+
+ lvds->pclk = devm_clk_get_prepared(lvds->dev, "pclk");
+ if (IS_ERR(lvds->pclk))
+ return dev_err_probe(lvds->dev, PTR_ERR(lvds->pclk),
+ "could not get pclk_lvds prepared\n");
+
+ port = of_graph_get_remote_node(dev->of_node, 1, 0);
+ if (!port) {
+ DRM_DEV_ERROR(dev,
+ "can't find port point, please init lvds panel port!\n");
+ return -ENODEV;
+ }
+
+ lvds->panel = of_drm_find_panel(port);
+ of_node_put(port);
+
+ if (IS_ERR(lvds->panel))
+ return -EPROBE_DEFER;
+
+ lvds->panel_bridge = devm_drm_panel_bridge_add(dev, lvds->panel);
+
+ if (IS_ERR(lvds->panel_bridge))
+ return PTR_ERR(lvds->panel_bridge);
+
+ lvds->bridge.of_node = dev->of_node;
+ lvds->bridge.type = DRM_MODE_CONNECTOR_LVDS;
+ lvds->bridge.funcs = &mchp_lvds_bridge_funcs;
+
+ dev_set_drvdata(dev, lvds);
+ devm_pm_runtime_enable(dev);
+
+ drm_bridge_add(&lvds->bridge);
+
+ return 0;
+}
+
+static const struct of_device_id mchp_lvds_dt_ids[] = {
+ {
+ .compatible = "microchip,sam9x75-lvds",
+ },
+ {},
+};
+MODULE_DEVICE_TABLE(of, mchp_lvds_dt_ids);
+
+static struct platform_driver mchp_lvds_driver = {
+ .probe = mchp_lvds_probe,
+ .driver = {
+ .name = "microchip-lvds",
+ .of_match_table = mchp_lvds_dt_ids,
+ },
+};
+module_platform_driver(mchp_lvds_driver);
+
+MODULE_AUTHOR("Manikandan Muralidharan <manikandan.m@microchip.com>");
+MODULE_AUTHOR("Dharma Balasubiramani <dharma.b@microchip.com>");
+MODULE_DESCRIPTION("Low Voltage Differential Signaling Controller Driver");
+MODULE_LICENSE("GPL");
--
2.25.1
^ permalink raw reply related
* [PATCH v5 3/4] MAINTAINERS: add SAM9X7 SoC's LVDS controller
From: Dharma Balasubiramani @ 2024-04-05 4:35 UTC (permalink / raw)
To: andrzej . hajda @ intel . com, neil . armstrong @ linaro . org,
rfoss @ kernel . org, Laurent . pinchart @ ideasonboard . com,
jonas @ kwiboo . se, jernej . skrabec @ gmail . com,
maarten . lankhorst @ linux . intel . com, mripard @ kernel . org,
tzimmermann @ suse . de, airlied @ gmail . com,
daniel @ ffwll . ch, robh+dt @ kernel . org,
krzysztof . kozlowski+dt @ linaro . org, conor+dt @ kernel . org,
linux @ armlinux . org . uk, Nicolas . Ferre @ microchip . com,
alexandre . belloni @ bootlin . com,
claudiu . beznea @ tuxon . dev, Manikandan . M @ microchip . com,
Dharma . B @ microchip . com, arnd @ arndb . de,
geert+renesas @ glider . be, Jason @ zx2c4 . com,
mpe @ ellerman . id . au, gerg @ linux-m68k . org,
rdunlap @ infradead . org, vbabka @ suse . cz,
dri-devel @ lists . freedesktop . org,
devicetree @ vger . kernel . org,
linux-kernel @ vger . kernel . org,
linux-arm-kernel @ lists . infradead . org,
Hari . PrasathGE @ microchip . com, akpm @ linux-foundation . org,
deller @ gmx . de
Cc: Dharma Balasubiramani, Nicolas Ferre
In-Reply-To: <20240405043536.274220-1-dharma.b@microchip.com>
Add the newly added LVDS controller for the SAM9X7 SoC to the existing
MAINTAINERS entry.
Signed-off-by: Dharma Balasubiramani <dharma.b@microchip.com>
Reviewed-by: Neil Armstrong <neil.armstrong@linaro.org>
Acked-by: Nicolas Ferre <nicolas.ferre@microchip.com>
---
Changelog
v4 -> v5
v3 -> v4
- No changes.
v2 -> v3
- Move the entry before "MICROCHIP SAMA5D2-COMPATIBLE ADC DRIVER".
v1 -> v2
- No Changes.
---
MAINTAINERS | 8 ++++++++
1 file changed, 8 insertions(+)
diff --git a/MAINTAINERS b/MAINTAINERS
index aa3b947fb080..3dd93dbe9542 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -14562,6 +14562,14 @@ S: Supported
F: Documentation/devicetree/bindings/pwm/atmel,at91sam-pwm.yaml
F: drivers/pwm/pwm-atmel.c
+MICROCHIP SAM9x7-COMPATIBLE LVDS CONTROLLER
+M: Manikandan Muralidharan <manikandan.m@microchip.com>
+M: Dharma Balasubiramani <dharma.b@microchip.com>
+L: dri-devel@lists.freedesktop.org
+S: Supported
+F: Documentation/devicetree/bindings/display/bridge/microchip,sam9x7-lvds.yaml
+F: drivers/gpu/drm/bridge/microchip-lvds.c
+
MICROCHIP SAMA5D2-COMPATIBLE ADC DRIVER
M: Eugen Hristev <eugen.hristev@microchip.com>
L: linux-iio@vger.kernel.org
--
2.25.1
^ permalink raw reply related
* [PATCH v5 4/4] ARM: configs: at91: Enable LVDS serializer support
From: Dharma Balasubiramani @ 2024-04-05 4:35 UTC (permalink / raw)
To: andrzej . hajda @ intel . com, neil . armstrong @ linaro . org,
rfoss @ kernel . org, Laurent . pinchart @ ideasonboard . com,
jonas @ kwiboo . se, jernej . skrabec @ gmail . com,
maarten . lankhorst @ linux . intel . com, mripard @ kernel . org,
tzimmermann @ suse . de, airlied @ gmail . com,
daniel @ ffwll . ch, robh+dt @ kernel . org,
krzysztof . kozlowski+dt @ linaro . org, conor+dt @ kernel . org,
linux @ armlinux . org . uk, Nicolas . Ferre @ microchip . com,
alexandre . belloni @ bootlin . com,
claudiu . beznea @ tuxon . dev, Manikandan . M @ microchip . com,
Dharma . B @ microchip . com, arnd @ arndb . de,
geert+renesas @ glider . be, Jason @ zx2c4 . com,
mpe @ ellerman . id . au, gerg @ linux-m68k . org,
rdunlap @ infradead . org, vbabka @ suse . cz,
dri-devel @ lists . freedesktop . org,
devicetree @ vger . kernel . org,
linux-kernel @ vger . kernel . org,
linux-arm-kernel @ lists . infradead . org,
Hari . PrasathGE @ microchip . com, akpm @ linux-foundation . org,
deller @ gmx . de
Cc: Dharma Balasubiramani, Hari Prasath Gujulan Elango, Nicolas Ferre
In-Reply-To: <20240405043536.274220-1-dharma.b@microchip.com>
Enable LVDS serializer support for display pipeline.
Signed-off-by: Dharma Balasubiramani <dharma.b@microchip.com>
Acked-by: Hari Prasath Gujulan Elango <hari.prasathge@microchip.com>
Acked-by: Nicolas Ferre <nicolas.ferre@microchip.com>
---
Changelog
v4 -> v5
v3 -> v4
v2 -> v3
- No Changes.
---
arch/arm/configs/at91_dt_defconfig | 1 +
1 file changed, 1 insertion(+)
diff --git a/arch/arm/configs/at91_dt_defconfig b/arch/arm/configs/at91_dt_defconfig
index 1d53aec4c836..6eabe2313c9a 100644
--- a/arch/arm/configs/at91_dt_defconfig
+++ b/arch/arm/configs/at91_dt_defconfig
@@ -143,6 +143,7 @@ CONFIG_VIDEO_OV2640=m
CONFIG_VIDEO_OV7740=m
CONFIG_DRM=y
CONFIG_DRM_ATMEL_HLCDC=y
+CONFIG_DRM_MICROCHIP_LVDS_SERIALIZER=y
CONFIG_DRM_PANEL_SIMPLE=y
CONFIG_DRM_PANEL_EDP=y
CONFIG_FB_ATMEL=y
--
2.25.1
^ permalink raw reply related
* Re: [PATCH v19 2/9] usb: dwc3: core: Access XHCI address space temporarily to read port info
From: Greg Kroah-Hartman @ 2024-04-05 4:43 UTC (permalink / raw)
To: Bjorn Andersson
Cc: Krzysztof Kozlowski, Johan Hovold, Krishna Kurapati,
Krzysztof Kozlowski, Rob Herring, Bjorn Andersson, Wesley Cheng,
Konrad Dybcio, Conor Dooley, Thinh Nguyen, Felipe Balbi,
devicetree, linux-arm-msm, linux-usb, linux-kernel, quic_ppratap,
quic_jackp, Johan Hovold
In-Reply-To: <Zg9THGBRuppfw4y+@hu-bjorande-lv.qualcomm.com>
On Thu, Apr 04, 2024 at 06:25:48PM -0700, Bjorn Andersson wrote:
> On Thu, Apr 04, 2024 at 02:58:29PM +0200, Greg Kroah-Hartman wrote:
> > On Thu, Apr 04, 2024 at 10:07:27AM +0200, Krzysztof Kozlowski wrote:
> > > On 04/04/2024 09:21, Johan Hovold wrote:
> > > > On Thu, Apr 04, 2024 at 10:42:22AM +0530, Krishna Kurapati wrote:
> > > >
> > > >> +static int dwc3_get_num_ports(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 (!base)
> > > >> + return PTR_ERR(base);
> > > >
> > > > This is obviously still broken. You need to update the return value as
> > > > well.
> > > >
> > > > Fix in v20.
> > >
> > > If one patchset reaches 20 versions, I think it is time to stop and
> > > really think from the beginning, why issues keep appearing and reviewers
> > > are still not happy.
> > >
> > > Maybe you did not perform extensive internal review, which you are
> > > encouraged to by your own internal policies, AFAIR. Before posting next
> > > version, please really get some internal review first.
> >
> > Also get those internal reviewers to sign-off on the commits and have
> > that show up when you post them next. That way they are also
> > responsible for this patchset, it's not fair that they are making you do
> > all the work here :)
> >
>
> I like this idea and I'm open to us changing our way of handling this.
>
> But unless such internal review brings significant input to the
> development I'd say a s-o-b would take the credit from the actual
> author.
It does not do that at all. It provides proof that someone else has
reviewed it and agrees with it. Think of it as a "path of blame" for
when things go bad (i.e. there is a bug in the submission.) Putting
your name on it makes you take responsibility if that happens.
> We've discussed a few times about carrying Reviewed-by et al from the
> internal reviews, but as maintainer I dislike this because I'd have no
> way to know if a r-b on vN means the patch was reviewed, or if it was
> just "accidentally" carried from v(N-1).
> But it might be worth this risk, is this something you think would be
> appropriate?
For some companies we REQUIRE this to happen due to low-quality
submissions and waste of reviewer's time. Based on the track record
here for some of these patchsets, hopefully it doesn't become a
requirement for this company as well :)
thanks,
greg k-h
^ permalink raw reply
* Re: [PATCH] dt-bindings: mfd: syscon: Add ti,am62p-cpsw-mac-efuse compatible
From: Siddharth Vadapalli @ 2024-04-05 5:21 UTC (permalink / raw)
To: Krzysztof Kozlowski
Cc: Siddharth Vadapalli, lee, robh, krzk+dt, conor+dt, devicetree,
linux-kernel, linux-arm-kernel, srk
In-Reply-To: <a895ddc8-5c18-49d7-86c4-b995bb946914@ti.com>
On Thu, Apr 04, 2024 at 02:02:21PM +0530, Siddharth Vadapalli wrote:
> On Wed, Apr 03, 2024 at 12:18:10PM +0530, Siddharth Vadapalli wrote:
> > On Wed, Apr 03, 2024 at 08:40:19AM +0200, Krzysztof Kozlowski wrote:
> > > On 03/04/2024 08:32, Siddharth Vadapalli wrote:
> > > > On Wed, Apr 03, 2024 at 08:27:06AM +0200, Krzysztof Kozlowski wrote:
> > > >> On 03/04/2024 07:35, Siddharth Vadapalli wrote:
> > > >>> On Tue, Apr 02, 2024 at 08:06:27PM +0200, Krzysztof Kozlowski wrote:
> > > >>>> On 02/04/2024 14:30, Siddharth Vadapalli wrote:
> > > >>>>> On Tue, Apr 02, 2024 at 02:08:32PM +0200, Krzysztof Kozlowski wrote:
> > > >>>>>> On 02/04/2024 12:57, Siddharth Vadapalli wrote:
> > > >>>>>>> The CTRLMMR_MAC_IDx registers within the CTRL_MMR space of TI's AM62p SoC
> > > >>>>>>> contain the MAC Address programmed in the eFuse. Add compatible for
> > > >>>>>>> allowing the CPSW driver to obtain a regmap for the CTRLMMR_MAC_IDx
> > > >>>>>>> registers within the System Controller device-tree node. The default MAC
> > > >>>>>>> Address for the interface corresponding to the first MAC port will be set
> > > >>>>>>> to the value programmed in the eFuse.
> > > >>>>>>>
> > > >>>>>>> Signed-off-by: Siddharth Vadapalli <s-vadapalli@ti.com>
> > > >>>>>>> ---
> > > >>>>>>>
> > > >>>>>>> This patch is based on linux-next tagged next-20240402.
> > > >>>>>>
> > > >>>>>> Where is the DTS using it?
> > > >>>>>
> > > >>>>> The current implementation in the device-tree for older TI K3 SoCs is as
> > > >>>>> follows:
> > > >>>>>
> > > >>>>> cpsw_port1: port@1 {
> > > >>>>> reg = <1>;
> > > >>>>> ti,mac-only;
> > > >>>>> label = "port1";
> > > >>>>> phys = <&phy_gmii_sel 1>;
> > > >>>>> mac-address = [00 00 00 00 00 00];
> > > >>>>> ti,syscon-efuse = <&wkup_conf 0x200>;
> > > >>>>> };
> > > >>>>>
> > > >>>>> The "ti,syscon-efuse" property passes the reference to the System
> > > >>>>> Controller node as well as the offset to the CTRLMMR_MAC_IDx registers
> > > >>>>> within the CTRL_MMR space.
> > > >>>>
> > > >>>> Please reference upstream DTS or lore link to patch under review.
> > > >>>
> > > >>> An example of the existing implementation in the device-tree for AM64x
> > > >>> is:
> > > >>> https://github.com/torvalds/linux/blob/d4e8c8ad5d14ad51ed8813442d81c43019fd669d/arch/arm64/boot/dts/ti/k3-am64-main.dtsi#L697
> > > >>> It uses:
> > > >>> ti,syscon-efuse = <&main_conf 0x200>;
> > > >>>
> > > >>> and "main_conf" node is defined at:
> > > >>> https://github.com/torvalds/linux/blob/d4e8c8ad5d14ad51ed8813442d81c43019fd669d/arch/arm64/boot/dts/ti/k3-am64-main.dtsi#L40
> > > >>
> > > >> It is quite different than your bindings, so your bindings are incorrect.
> > > >
> > > > Sorry I didn't understand what you mean. The references I have provided
> > > > are for existing DTS where "main_conf"/"wkup_conf" (System Controller
> > > > nodes) have the compatible "syscon", unlike in AM62p at:
> > > > https://github.com/torvalds/linux/blob/20f8173afaac90dd9dca11be4aa602a47776077f/arch/arm64/boot/dts/ti/k3-am62p-wakeup.dtsi#L8
> > > > which has the "simple-bus" compatible for the "wkup_conf" node.
> > > >
> > > > Also, shouldn't the device-tree bindings patches be posted first and get
> > > > merged before I post the device-tree patches that utilize the
> > > > compatible/properties that have been added in the bindings? That is the
> > > > reason why I had shared the "DIFF" for the DTS changes that I will be
> > > > posting once this patch for the new compatible is accepted.
> > > >
> > >
> > > That's not the process. I will be NAKing bindings which do not have any
> > > users, because I do not trust you test them.
> > >
> > > The process is almost always:
> > > 1. Send bindings,
> > > 2. Send driver changes (if applicable) in the same patchset.
> > > 3. Send DTS, usually in separate patches and provide lore link to the
> > > bindings in the changelog or cover letter.
> >
> > Thank you for clarifying. I will post the DTS patches corresponding to
> > this patch and reference this patch in the DTS patch series.
>
> I have posted the DTS patch at:
> https://patchwork.kernel.org/project/linux-arm-kernel/patch/20240404081845.622707-1-s-vadapalli@ti.com/
> indicating the dependency on this bindings patch.
Hello Krzysztof,
Do I have to post a v2 for this patch? You had Acked it initially but I
am not sure if the discussion so far will make it unclear to readers
regarding the acceptance of this patch. Thank you for Acking the v3 DTS
patch at:
https://patchwork.kernel.org/project/linux-arm-kernel/patch/20240404124614.891416-1-s-vadapalli@ti.com/
Since the v3 DTS patch mentions this bindings patch as a dependency, I
wanted to be sure whether I have to post a v2 for this or that won't be
required.
Regards,
Siddharth.
^ permalink raw reply
* Re: [RFC PATCH 2/2] PCI: Add Qualcomm PCIe ECAM root complex driver
From: Manivannan Sadhasivam @ 2024-04-05 5:30 UTC (permalink / raw)
To: Mayank Rana
Cc: linux-pci, lpieralisi, kw, robh, bhelgaas, andersson,
krzysztof.kozlowski+dt, conor+dt, devicetree, linux-arm-msm,
quic_ramkri, quic_nkela, quic_shazhuss, quic_msarkar,
quic_nitegupt
In-Reply-To: <1712257884-23841-3-git-send-email-quic_mrana@quicinc.com>
On Thu, Apr 04, 2024 at 12:11:24PM -0700, Mayank Rana wrote:
> On some of Qualcomm platform, firmware configures PCIe controller into
> ECAM mode allowing static memory allocation for configuration space of
> supported bus range. Firmware also takes care of bringing up PCIe PHY
> and performing required operation to bring PCIe link into D0. Firmware
> also manages system resources (e.g. clocks/regulators/resets/ bus voting).
> Hence add Qualcomm PCIe ECAM root complex driver which enumerates PCIe
> root complex and connected PCIe devices. Firmware won't be enumerating
> or powering up PCIe root complex until this driver invokes power domain
> based notification to bring PCIe link into D0/D3cold mode.
>
Is this an in-house PCIe IP of Qualcomm or the same DWC IP that is used in other
SoCs?
- Mani
> This driver also support MSI functionality using PCIe controller based
> MSI controller as GIC ITS based MSI functionality is not available on
> some of platform.
>
> Signed-off-by: Mayank Rana <quic_mrana@quicinc.com>
> ---
> drivers/pci/controller/Kconfig | 12 +
> drivers/pci/controller/Makefile | 1 +
> drivers/pci/controller/pcie-qcom-ecam.c | 575 ++++++++++++++++++++++++++++++++
> 3 files changed, 588 insertions(+)
> create mode 100644 drivers/pci/controller/pcie-qcom-ecam.c
>
> diff --git a/drivers/pci/controller/Kconfig b/drivers/pci/controller/Kconfig
> index e534c02..abbd9f2 100644
> --- a/drivers/pci/controller/Kconfig
> +++ b/drivers/pci/controller/Kconfig
> @@ -353,6 +353,18 @@ config PCIE_XILINX_CPM
> Say 'Y' here if you want kernel support for the
> Xilinx Versal CPM host bridge.
>
> +config PCIE_QCOM_ECAM
> + tristate "QCOM PCIe ECAM host controller"
> + depends on ARCH_QCOM && PCI
> + depends on OF
> + select PCI_MSI
> + select PCI_HOST_COMMON
> + select IRQ_DOMAIN
> + help
> + Say 'Y' here if you want to use ECAM shift mode compatible Qualcomm
> + PCIe root host controller. The controller is programmed using firmware
> + to support ECAM compatible memory address space.
> +
> source "drivers/pci/controller/cadence/Kconfig"
> source "drivers/pci/controller/dwc/Kconfig"
> source "drivers/pci/controller/mobiveil/Kconfig"
> diff --git a/drivers/pci/controller/Makefile b/drivers/pci/controller/Makefile
> index f2b19e6..2f1ee1e 100644
> --- a/drivers/pci/controller/Makefile
> +++ b/drivers/pci/controller/Makefile
> @@ -40,6 +40,7 @@ obj-$(CONFIG_PCI_LOONGSON) += pci-loongson.o
> obj-$(CONFIG_PCIE_HISI_ERR) += pcie-hisi-error.o
> obj-$(CONFIG_PCIE_APPLE) += pcie-apple.o
> obj-$(CONFIG_PCIE_MT7621) += pcie-mt7621.o
> +obj-$(CONFIG_PCIE_QCOM_ECAM) += pcie-qcom-ecam.o
>
> # pcie-hisi.o quirks are needed even without CONFIG_PCIE_DW
> obj-y += dwc/
> diff --git a/drivers/pci/controller/pcie-qcom-ecam.c b/drivers/pci/controller/pcie-qcom-ecam.c
> new file mode 100644
> index 00000000..5b4c68b
> --- /dev/null
> +++ b/drivers/pci/controller/pcie-qcom-ecam.c
> @@ -0,0 +1,575 @@
> +// SPDX-License-Identifier: GPL-2.0-only
> +/*
> + * Qualcomm PCIe ECAM root host controller driver
> + * Copyright (c) 2024 Qualcomm Innovation Center, Inc. All rights reserved.
> + */
> +#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
> +
> +#include <linux/irq.h>
> +#include <linux/irqchip/chained_irq.h>
> +#include <linux/irqdomain.h>
> +#include <linux/kernel.h>
> +#include <linux/module.h>
> +#include <linux/msi.h>
> +#include <linux/of_address.h>
> +#include <linux/of_irq.h>
> +#include <linux/of_pci.h>
> +#include <linux/pci.h>
> +#include <linux/pci-ecam.h>
> +#include <linux/platform_device.h>
> +#include <linux/pm_domain.h>
> +#include <linux/pm_runtime.h>
> +#include <linux/slab.h>
> +#include <linux/types.h>
> +
> +#define PCIE_MSI_CTRL_BASE (0x820)
> +#define PCIE_MSI_CTRL_SIZE (0x68)
> +#define PCIE_MSI_CTRL_ADDR_OFFS (0x0)
> +#define PCIE_MSI_CTRL_UPPER_ADDR_OFFS (0x4)
> +#define PCIE_MSI_CTRL_INT_N_EN_OFFS(n) (0x8 + 0xc * (n))
> +#define PCIE_MSI_CTRL_INT_N_MASK_OFFS(n) (0xc + 0xc * (n))
> +#define PCIE_MSI_CTRL_INT_N_STATUS_OFFS(n) (0x10 + 0xc * (n))
> +
> +#define MSI_DB_ADDR 0xa0000000
> +#define MSI_IRQ_PER_GRP (32)
> +
> +/**
> + * struct qcom_msi_irq - MSI IRQ information
> + * @client: pointer to MSI client struct
> + * @grp: group the irq belongs to
> + * @grp_index: index in group
> + * @hwirq: hwirq number
> + * @virq: virq number
> + * @pos: position in MSI bitmap
> + */
> +struct qcom_msi_irq {
> + struct qcom_msi_client *client;
> + struct qcom_msi_grp *grp;
> + unsigned int grp_index;
> + unsigned int hwirq;
> + unsigned int virq;
> + u32 pos;
> +};
> +
> +/**
> + * struct qcom_msi_grp - MSI group information
> + * @int_en_reg: memory-mapped interrupt enable register address
> + * @int_mask_reg: memory-mapped interrupt mask register address
> + * @int_status_reg: memory-mapped interrupt status register address
> + * @mask: tracks masked/unmasked MSI
> + * @irqs: structure to MSI IRQ information
> + */
> +struct qcom_msi_grp {
> + void __iomem *int_en_reg;
> + void __iomem *int_mask_reg;
> + void __iomem *int_status_reg;
> + u32 mask;
> + struct qcom_msi_irq irqs[MSI_IRQ_PER_GRP];
> +};
> +
> +/**
> + * struct qcom_msi - PCIe controller based MSI controller information
> + * @clients: list for tracking clients
> + * @dev: platform device node
> + * @nr_hwirqs: total number of hardware IRQs
> + * @nr_virqs: total number of virqs
> + * @nr_grps: total number of groups
> + * @grps: pointer to all groups information
> + * @bitmap: tracks used/unused MSI
> + * @mutex: for modifying MSI client list and bitmap
> + * @inner_domain: parent domain; gen irq related
> + * @msi_domain: child domain; pcie related
> + * @msi_db_addr: MSI doorbell address
> + * @cfg_lock: lock for configuring MSI controller registers
> + * @pcie_msi_cfg: memory-mapped MSI controller register space
> + */
> +struct qcom_msi {
> + struct list_head clients;
> + struct device *dev;
> + int nr_hwirqs;
> + int nr_virqs;
> + int nr_grps;
> + struct qcom_msi_grp *grps;
> + unsigned long *bitmap;
> + struct mutex mutex;
> + struct irq_domain *inner_domain;
> + struct irq_domain *msi_domain;
> + phys_addr_t msi_db_addr;
> + spinlock_t cfg_lock;
> + void __iomem *pcie_msi_cfg;
> +};
> +
> +/**
> + * struct qcom_msi_client - structure for each client of MSI controller
> + * @node: list to track number of MSI clients
> + * @msi: client specific MSI controller based resource pointer
> + * @dev: client's dev of pci_dev
> + * @nr_irqs: number of irqs allocated for client
> + * @msi_addr: MSI doorbell address
> + */
> +struct qcom_msi_client {
> + struct list_head node;
> + struct qcom_msi *msi;
> + struct device *dev;
> + unsigned int nr_irqs;
> + phys_addr_t msi_addr;
> +};
> +
> +static void qcom_msi_handler(struct irq_desc *desc)
> +{
> + struct irq_chip *chip = irq_desc_get_chip(desc);
> + struct qcom_msi_grp *msi_grp;
> + u32 status;
> + int i;
> +
> + chained_irq_enter(chip, desc);
> +
> + msi_grp = irq_desc_get_handler_data(desc);
> + status = readl_relaxed(msi_grp->int_status_reg);
> + status ^= (msi_grp->mask & status);
> + writel(status, msi_grp->int_status_reg);
> +
> + for (i = 0; status; i++, status >>= 1)
> + if (status & 0x1)
> + generic_handle_irq(msi_grp->irqs[i].virq);
> +
> + chained_irq_exit(chip, desc);
> +}
> +
> +static void qcom_msi_mask_irq(struct irq_data *data)
> +{
> + struct irq_data *parent_data;
> + struct qcom_msi_irq *msi_irq;
> + struct qcom_msi_grp *msi_grp;
> + struct qcom_msi *msi;
> + unsigned long flags;
> +
> + parent_data = data->parent_data;
> + if (!parent_data)
> + return;
> +
> + msi_irq = irq_data_get_irq_chip_data(parent_data);
> + msi = msi_irq->client->msi;
> + msi_grp = msi_irq->grp;
> +
> + spin_lock_irqsave(&msi->cfg_lock, flags);
> + pci_msi_mask_irq(data);
> + msi_grp->mask |= BIT(msi_irq->grp_index);
> + writel(msi_grp->mask, msi_grp->int_mask_reg);
> + spin_unlock_irqrestore(&msi->cfg_lock, flags);
> +}
> +
> +static void qcom_msi_unmask_irq(struct irq_data *data)
> +{
> + struct irq_data *parent_data;
> + struct qcom_msi_irq *msi_irq;
> + struct qcom_msi_grp *msi_grp;
> + struct qcom_msi *msi;
> + unsigned long flags;
> +
> + parent_data = data->parent_data;
> + if (!parent_data)
> + return;
> +
> + msi_irq = irq_data_get_irq_chip_data(parent_data);
> + msi = msi_irq->client->msi;
> + msi_grp = msi_irq->grp;
> +
> + spin_lock_irqsave(&msi->cfg_lock, flags);
> + msi_grp->mask &= ~BIT(msi_irq->grp_index);
> + writel(msi_grp->mask, msi_grp->int_mask_reg);
> + pci_msi_unmask_irq(data);
> + spin_unlock_irqrestore(&msi->cfg_lock, flags);
> +}
> +
> +static struct irq_chip qcom_msi_irq_chip = {
> + .name = "qcom_pci_msi",
> + .irq_enable = qcom_msi_unmask_irq,
> + .irq_disable = qcom_msi_mask_irq,
> + .irq_mask = qcom_msi_mask_irq,
> + .irq_unmask = qcom_msi_unmask_irq,
> +};
> +
> +static int qcom_msi_domain_prepare(struct irq_domain *domain, struct device *dev,
> + int nvec, msi_alloc_info_t *arg)
> +{
> + struct qcom_msi *msi = domain->parent->host_data;
> + struct qcom_msi_client *client;
> +
> + client = kzalloc(sizeof(*client), GFP_KERNEL);
> + if (!client)
> + return -ENOMEM;
> +
> + client->msi = msi;
> + client->dev = dev;
> + client->msi_addr = msi->msi_db_addr;
> + mutex_lock(&msi->mutex);
> + list_add_tail(&client->node, &msi->clients);
> + mutex_unlock(&msi->mutex);
> +
> + /* zero out struct for pcie msi framework */
> + memset(arg, 0, sizeof(*arg));
> + return 0;
> +}
> +
> +static struct msi_domain_ops qcom_msi_domain_ops = {
> + .msi_prepare = qcom_msi_domain_prepare,
> +};
> +
> +static struct msi_domain_info qcom_msi_domain_info = {
> + .flags = (MSI_FLAG_USE_DEF_DOM_OPS | MSI_FLAG_USE_DEF_CHIP_OPS |
> + MSI_FLAG_MULTI_PCI_MSI | MSI_FLAG_PCI_MSIX),
> + .ops = &qcom_msi_domain_ops,
> + .chip = &qcom_msi_irq_chip,
> +};
> +
> +static int qcom_msi_irq_set_affinity(struct irq_data *data,
> + const struct cpumask *mask, bool force)
> +{
> + struct irq_data *parent_data = irq_get_irq_data(irqd_to_hwirq(data));
> + int ret = 0;
> +
> + if (!parent_data)
> + return -ENODEV;
> +
> + /* set affinity for MSI HW IRQ */
> + if (parent_data->chip->irq_set_affinity)
> + ret = parent_data->chip->irq_set_affinity(parent_data, mask, force);
> +
> + return ret;
> +}
> +
> +static void qcom_msi_irq_compose_msi_msg(struct irq_data *data, struct msi_msg *msg)
> +{
> + struct irq_data *parent_data = irq_get_irq_data(irqd_to_hwirq(data));
> + struct qcom_msi_irq *msi_irq = irq_data_get_irq_chip_data(data);
> + struct qcom_msi_client *client = msi_irq->client;
> +
> + if (!parent_data)
> + return;
> +
> + msg->address_lo = lower_32_bits(client->msi_addr);
> + msg->address_hi = upper_32_bits(client->msi_addr);
> + msg->data = msi_irq->pos;
> +}
> +
> +static struct irq_chip qcom_msi_bottom_irq_chip = {
> + .name = "qcom_msi",
> + .irq_set_affinity = qcom_msi_irq_set_affinity,
> + .irq_compose_msi_msg = qcom_msi_irq_compose_msi_msg,
> +};
> +
> +static int qcom_msi_irq_domain_alloc(struct irq_domain *domain, unsigned int virq,
> + unsigned int nr_irqs, void *args)
> +{
> + struct device *dev = ((msi_alloc_info_t *)args)->desc->dev;
> + struct qcom_msi_client *tmp, *client = NULL;
> + struct qcom_msi *msi = domain->host_data;
> + int i, ret = 0;
> + int pos;
> +
> + mutex_lock(&msi->mutex);
> + list_for_each_entry(tmp, &msi->clients, node) {
> + if (tmp->dev == dev) {
> + client = tmp;
> + break;
> + }
> + }
> +
> + if (!client) {
> + dev_err(msi->dev, "failed to find MSI client dev\n");
> + ret = -ENODEV;
> + goto out;
> + }
> +
> + pos = bitmap_find_next_zero_area(msi->bitmap, msi->nr_virqs, 0,
> + nr_irqs, nr_irqs - 1);
> + if (pos > msi->nr_virqs) {
> + ret = -ENOSPC;
> + goto out;
> + }
> +
> + bitmap_set(msi->bitmap, pos, nr_irqs);
> + for (i = 0; i < nr_irqs; i++) {
> + u32 grp = pos / MSI_IRQ_PER_GRP;
> + u32 index = pos % MSI_IRQ_PER_GRP;
> + struct qcom_msi_irq *msi_irq = &msi->grps[grp].irqs[index];
> +
> + msi_irq->virq = virq + i;
> + msi_irq->client = client;
> + irq_domain_set_info(domain, msi_irq->virq,
> + msi_irq->hwirq,
> + &qcom_msi_bottom_irq_chip, msi_irq,
> + handle_simple_irq, NULL, NULL);
> + client->nr_irqs++;
> + pos++;
> + }
> +out:
> + mutex_unlock(&msi->mutex);
> + return ret;
> +}
> +
> +static void qcom_msi_irq_domain_free(struct irq_domain *domain, unsigned int virq,
> + unsigned int nr_irqs)
> +{
> + struct irq_data *data = irq_domain_get_irq_data(domain, virq);
> + struct qcom_msi_client *client;
> + struct qcom_msi_irq *msi_irq;
> + struct qcom_msi *msi;
> +
> + if (!data)
> + return;
> +
> + msi_irq = irq_data_get_irq_chip_data(data);
> + client = msi_irq->client;
> + msi = client->msi;
> +
> + mutex_lock(&msi->mutex);
> + bitmap_clear(msi->bitmap, msi_irq->pos, nr_irqs);
> +
> + client->nr_irqs -= nr_irqs;
> + if (!client->nr_irqs) {
> + list_del(&client->node);
> + kfree(client);
> + }
> + mutex_unlock(&msi->mutex);
> +
> + irq_domain_free_irqs_parent(domain, virq, nr_irqs);
> +}
> +
> +static const struct irq_domain_ops msi_domain_ops = {
> + .alloc = qcom_msi_irq_domain_alloc,
> + .free = qcom_msi_irq_domain_free,
> +};
> +
> +static int qcom_msi_alloc_domains(struct qcom_msi *msi)
> +{
> + msi->inner_domain = irq_domain_add_linear(NULL, msi->nr_virqs,
> + &msi_domain_ops, msi);
> + if (!msi->inner_domain) {
> + dev_err(msi->dev, "failed to create IRQ inner domain\n");
> + return -ENOMEM;
> + }
> +
> + msi->msi_domain = pci_msi_create_irq_domain(of_node_to_fwnode(msi->dev->of_node),
> + &qcom_msi_domain_info, msi->inner_domain);
> + if (!msi->msi_domain) {
> + dev_err(msi->dev, "failed to create MSI domain\n");
> + irq_domain_remove(msi->inner_domain);
> + return -ENOMEM;
> + }
> +
> + return 0;
> +}
> +
> +static int qcom_msi_irq_setup(struct qcom_msi *msi)
> +{
> + struct qcom_msi_grp *msi_grp;
> + struct qcom_msi_irq *msi_irq;
> + int i, index, ret;
> + unsigned int irq;
> +
> + /* setup each MSI group. nr_hwirqs == nr_grps */
> + for (i = 0; i < msi->nr_hwirqs; i++) {
> + irq = irq_of_parse_and_map(msi->dev->of_node, i);
> + if (!irq) {
> + dev_err(msi->dev,
> + "MSI: failed to parse/map interrupt\n");
> + ret = -ENODEV;
> + goto free_irqs;
> + }
> +
> + msi_grp = &msi->grps[i];
> + msi_grp->int_en_reg = msi->pcie_msi_cfg +
> + PCIE_MSI_CTRL_INT_N_EN_OFFS(i);
> + msi_grp->int_mask_reg = msi->pcie_msi_cfg +
> + PCIE_MSI_CTRL_INT_N_MASK_OFFS(i);
> + msi_grp->int_status_reg = msi->pcie_msi_cfg +
> + PCIE_MSI_CTRL_INT_N_STATUS_OFFS(i);
> +
> + for (index = 0; index < MSI_IRQ_PER_GRP; index++) {
> + msi_irq = &msi_grp->irqs[index];
> +
> + msi_irq->grp = msi_grp;
> + msi_irq->grp_index = index;
> + msi_irq->pos = (i * MSI_IRQ_PER_GRP) + index;
> + msi_irq->hwirq = irq;
> + }
> +
> + irq_set_chained_handler_and_data(irq, qcom_msi_handler, msi_grp);
> + }
> +
> + return 0;
> +
> +free_irqs:
> + for (--i; i >= 0; i--) {
> + irq = msi->grps[i].irqs[0].hwirq;
> +
> + irq_set_chained_handler_and_data(irq, NULL, NULL);
> + irq_dispose_mapping(irq);
> + }
> +
> + return ret;
> +}
> +
> +static void qcom_msi_config(struct irq_domain *domain)
> +{
> + struct qcom_msi *msi;
> + int i;
> +
> + msi = domain->parent->host_data;
> +
> + /* program termination address */
> + writel(msi->msi_db_addr, msi->pcie_msi_cfg + PCIE_MSI_CTRL_ADDR_OFFS);
> + writel(0, msi->pcie_msi_cfg + PCIE_MSI_CTRL_UPPER_ADDR_OFFS);
> +
> + /* restore mask and enable all interrupts for each group */
> + for (i = 0; i < msi->nr_grps; i++) {
> + struct qcom_msi_grp *msi_grp = &msi->grps[i];
> +
> + writel(msi_grp->mask, msi_grp->int_mask_reg);
> + writel(~0, msi_grp->int_en_reg);
> + }
> +}
> +
> +static void qcom_msi_deinit(struct qcom_msi *msi)
> +{
> + irq_domain_remove(msi->msi_domain);
> + irq_domain_remove(msi->inner_domain);
> +}
> +
> +static struct qcom_msi *qcom_msi_init(struct device *dev)
> +{
> + struct qcom_msi *msi;
> + u64 addr;
> + int ret;
> +
> + msi = devm_kzalloc(dev, sizeof(*msi), GFP_KERNEL);
> + if (!msi)
> + return ERR_PTR(-ENOMEM);
> +
> + msi->dev = dev;
> + mutex_init(&msi->mutex);
> + spin_lock_init(&msi->cfg_lock);
> + INIT_LIST_HEAD(&msi->clients);
> +
> + msi->msi_db_addr = MSI_DB_ADDR;
> + msi->nr_hwirqs = of_irq_count(dev->of_node);
> + if (!msi->nr_hwirqs) {
> + dev_err(msi->dev, "no hwirqs found\n");
> + return ERR_PTR(-ENODEV);
> + }
> +
> + if (of_property_read_reg(dev->of_node, 0, &addr, NULL) < 0) {
> + dev_err(msi->dev, "failed to get reg address\n");
> + return ERR_PTR(-ENODEV);
> + }
> +
> + dev_dbg(msi->dev, "hwirq:%d pcie_msi_cfg:%llx\n", msi->nr_hwirqs, addr);
> + msi->pcie_msi_cfg = devm_ioremap(dev, addr + PCIE_MSI_CTRL_BASE, PCIE_MSI_CTRL_SIZE);
> + if (!msi->pcie_msi_cfg)
> + return ERR_PTR(-ENOMEM);
> +
> + msi->nr_virqs = msi->nr_hwirqs * MSI_IRQ_PER_GRP;
> + msi->nr_grps = msi->nr_hwirqs;
> + msi->grps = devm_kcalloc(dev, msi->nr_grps, sizeof(*msi->grps), GFP_KERNEL);
> + if (!msi->grps)
> + return ERR_PTR(-ENOMEM);
> +
> + msi->bitmap = devm_kcalloc(dev, BITS_TO_LONGS(msi->nr_virqs),
> + sizeof(*msi->bitmap), GFP_KERNEL);
> + if (!msi->bitmap)
> + return ERR_PTR(-ENOMEM);
> +
> + ret = qcom_msi_alloc_domains(msi);
> + if (ret)
> + return ERR_PTR(ret);
> +
> + ret = qcom_msi_irq_setup(msi);
> + if (ret) {
> + qcom_msi_deinit(msi);
> + return ERR_PTR(ret);
> + }
> +
> + qcom_msi_config(msi->msi_domain);
> + return msi;
> +}
> +
> +static int qcom_pcie_ecam_suspend_noirq(struct device *dev)
> +{
> + return pm_runtime_put_sync(dev);
> +}
> +
> +static int qcom_pcie_ecam_resume_noirq(struct device *dev)
> +{
> + return pm_runtime_get_sync(dev);
> +}
> +
> +static int qcom_pcie_ecam_probe(struct platform_device *pdev)
> +{
> + struct device *dev = &pdev->dev;
> + struct qcom_msi *msi;
> + int ret;
> +
> + ret = devm_pm_runtime_enable(dev);
> + if (ret)
> + return ret;
> +
> + ret = pm_runtime_resume_and_get(dev);
> + if (ret < 0) {
> + dev_err(dev, "fail to enable pcie controller: %d\n", ret);
> + return ret;
> + }
> +
> + msi = qcom_msi_init(dev);
> + if (IS_ERR(msi)) {
> + pm_runtime_put_sync(dev);
> + return PTR_ERR(msi);
> + }
> +
> + ret = pci_host_common_probe(pdev);
> + if (ret) {
> + dev_err(dev, "pci_host_common_probe() failed:%d\n", ret);
> + qcom_msi_deinit(msi);
> + pm_runtime_put_sync(dev);
> + }
> +
> + return ret;
> +}
> +
> +static const struct dev_pm_ops qcom_pcie_ecam_pm_ops = {
> + NOIRQ_SYSTEM_SLEEP_PM_OPS(qcom_pcie_ecam_suspend_noirq,
> + qcom_pcie_ecam_resume_noirq)
> +};
> +
> +static const struct pci_ecam_ops qcom_pcie_ecam_ops = {
> + .pci_ops = {
> + .map_bus = pci_ecam_map_bus,
> + .read = pci_generic_config_read,
> + .write = pci_generic_config_write,
> + }
> +};
> +
> +static const struct of_device_id qcom_pcie_ecam_of_match[] = {
> + {
> + .compatible = "qcom,pcie-ecam-rc",
> + .data = &qcom_pcie_ecam_ops,
> + },
> + { },
> +};
> +MODULE_DEVICE_TABLE(of, qcom_pcie_ecam_of_match);
> +
> +static struct platform_driver qcom_pcie_ecam_driver = {
> + .probe = qcom_pcie_ecam_probe,
> + .driver = {
> + .name = "qcom-pcie-ecam-rc",
> + .suppress_bind_attrs = true,
> + .of_match_table = qcom_pcie_ecam_of_match,
> + .probe_type = PROBE_PREFER_ASYNCHRONOUS,
> + .pm = &qcom_pcie_ecam_pm_ops,
> + },
> +};
> +module_platform_driver(qcom_pcie_ecam_driver);
> +
> +MODULE_DESCRIPTION("Qualcomm PCIe ECAM root complex driver");
> +MODULE_LICENSE("GPL");
> --
> 2.7.4
>
--
மணிவண்ணன் சதாசிவம்
^ permalink raw reply
* Re: [PATCH v2 1/2] ASoC: dt-bindings: qcom,sm8250: Add QCM6490 snd QCS6490 sound card
From: Mohammad Rafi Shaik @ 2024-04-05 5:55 UTC (permalink / raw)
To: Dmitry Baryshkov
Cc: Srinivas Kandagatla, Banajit Goswami, Liam Girdwood, Mark Brown,
Rob Herring, Krzysztof Kozlowski, Conor Dooley, Jaroslav Kysela,
Takashi Iwai, linux-arm-msm, alsa-devel, linux-sound, devicetree,
linux-kernel, quic_rohkumar
In-Reply-To: <CAA8EJpqWaYhzPKgTREtJnfdNZ4oSFZaRFM7Jhg+qd3AqadGOkA@mail.gmail.com>
On 4/4/2024 2:23 PM, Dmitry Baryshkov wrote:
> On Thu, 4 Apr 2024 at 11:48, Mohammad Rafi Shaik <quic_mohs@quicinc.com> wrote:
>>
>> Document the bindings for the Qualcomm QCM6490 IDP and QCS6490 RB3Gen2
>> soc platforms sound card.
>>
>> The bindings are the same as for other newer Qualcomm ADSP sound cards,
>> thus keep them in existing qcom,sm8250.yaml file, even though Linux driver
>> is separate.
>>
>> Signed-off-by: Mohammad Rafi Shaik <quic_mohs@quicinc.com>
>> ---
>> Documentation/devicetree/bindings/sound/qcom,sm8250.yaml | 2 ++
>> 1 file changed, 2 insertions(+)
>>
>> diff --git a/Documentation/devicetree/bindings/sound/qcom,sm8250.yaml b/Documentation/devicetree/bindings/sound/qcom,sm8250.yaml
>> index 2ab6871e89e5..ff1a27f26bc2 100644
>> --- a/Documentation/devicetree/bindings/sound/qcom,sm8250.yaml
>> +++ b/Documentation/devicetree/bindings/sound/qcom,sm8250.yaml
>> @@ -29,6 +29,8 @@ properties:
>> - enum:
>> - qcom,apq8016-sbc-sndcard
>> - qcom,msm8916-qdsp6-sndcard
>> + - qcom,qcm6490-sndcard
>> + - qcom,qcs6490-rb3gen2-sndcard
>
> My 2c: you are adding one soundcard for the SoC family (qcm6490) and
> another one for the particular board kind (qcs6490-rb3gen2). That
> doesn't seem logical.
The qcm6490-sndcard compatible for enabling soundcard on
qcm6490 IDP boards.
Will change compatible name as qcom,qcm6490-idp-sndcard.
Thanks,
Rafi.
>
>> - qcom,qrb5165-rb5-sndcard
>> - qcom,sc7180-qdsp6-sndcard
>> - qcom,sc8280xp-sndcard
>> --
>> 2.25.1
>>
>>
>
>
^ permalink raw reply
* Re: [PATCH V2 RESEND 6/6] arm64: dts: qcom: sm8650: Add video and camera clock controllers
From: Jagadeesh Kona @ 2024-04-05 6:00 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: <CAA8EJpoW8MQQ3OPfOVYRJtgsn1JgKd5Ew7vqgWx3xWE-xJ=R-g@mail.gmail.com>
On 4/4/2024 9:35 PM, Dmitry Baryshkov wrote:
> On Thu, 4 Apr 2024 at 13:06, Jagadeesh Kona <quic_jkona@quicinc.com> wrote:
>>
>>
>>
>> On 4/4/2024 11:00 AM, Dmitry Baryshkov wrote:
>>> On Thu, 4 Apr 2024 at 08:13, Jagadeesh Kona <quic_jkona@quicinc.com> wrote:
>>>>
>>>>
>>>>
>>>> On 4/3/2024 9:24 PM, Dmitry Baryshkov wrote:
>>>>> On Wed, 3 Apr 2024 at 10:16, Jagadeesh Kona <quic_jkona@quicinc.com> wrote:
>>>>>>
>>>>>>
>>>>>>
>>>>>> 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.
>>>>>
>>>>> In which cases? If it ends up with the retention vote, it is a bug
>>>>> which must be fixed.
>>>>>
>>>>
>>>> The minimum non-zero level(configured from bootloaders) of MMCX is
>>>> retention on few chipsets but it can vary across the chipsets. Hence to
>>>> be on safer side from our end, it is good to have required-opps in DT to
>>>> specify the minimum level required for this clock controller.
>>>
>>> We are discussing sm8650, not some abstract chipset. Does it list
>>> retention or low_svs as a minimal level for MMCX?
>>>
>>
>> Actually, the minimum level for MMCX is external to the clock
>> controllers.
>
> Yes, it comes from cmd-db
>
>> But the clock controller requires MMCX to be atleast at
>> lowsvs for it to be functional.
>
> Correct
>
>> Hence we need to keep required-opps to
>> ensure the same without relying on the actual minimum level for MMCX.
>
> And this is not correct. There is no need for the DT to be redundant.
> I plan to send patches removing the existing required-opps when they
> are not required.
>
I agree this is not required if cmd-db minimum level is already at
lowsvs. But since MMCX running at lowsvs is a mandatory requirement for
clock controller to operate, I believe it is good to have required-opps
to ensure we meet this requirement in all cases, rather than relying on
the cmd-db minimum level which we have no control over.
Thanks,
Jagadeesh
^ permalink raw reply
* Re: [PATCH 1/2] dt-bindings: mfd: Add ROHM BD71879
From: Matti Vaittinen @ 2024-04-05 6:01 UTC (permalink / raw)
To: Krzysztof Kozlowski, Andreas Kemnade
Cc: lee, robh+dt, krzysztof.kozlowski+dt, conor+dt, devicetree,
linux-kernel
In-Reply-To: <8f37211a-57ed-48ab-8de8-cd5a0d4c6609@linaro.org>
On 4/4/24 15:04, Krzysztof Kozlowski wrote:
> On 04/04/2024 12:30, Andreas Kemnade wrote:
>> On Thu, 4 Apr 2024 08:59:54 +0200
>> Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org> wrote:
>>
>>> On 02/04/2024 21:35, Andreas Kemnade wrote:
>>>> As this chip was seen in several devices in the wild, add it.
>>>>
>>>> Signed-off-by: Andreas Kemnade <andreas@kemnade.info>
>>>> Suggested-by: Matti Vaittinen <mazziesaccount@gmail.com>
>>>> ---
>>>> Documentation/devicetree/bindings/mfd/rohm,bd71828-pmic.yaml | 4 +++-
>>>> 1 file changed, 3 insertions(+), 1 deletion(-)
>>>>
>>>> diff --git a/Documentation/devicetree/bindings/mfd/rohm,bd71828-pmic.yaml b/Documentation/devicetree/bindings/mfd/rohm,bd71828-pmic.yaml
>>>> index 0b62f854bf6b..e4df09e8961c 100644
>>>> --- a/Documentation/devicetree/bindings/mfd/rohm,bd71828-pmic.yaml
>>>> +++ b/Documentation/devicetree/bindings/mfd/rohm,bd71828-pmic.yaml
>>>> @@ -17,7 +17,9 @@ description: |
>>>>
>>>> properties:
>>>> compatible:
>>>> - const: rohm,bd71828
>>>> + enum:
>>>> + - rohm,bd71828
>>>> + - rohm,bd71879
>>>
>>> In your second commit you claim they are compatible, so why they are not
>>> marked as such?
>>>
>> so you mean allowing
>>
>> compatible = "rohm,bd71828"
>> and
>> compatible = "rohm,bd71879", "rohm,bd71828"
This makes me slightly nervous. It wouldn't be the first time when I've
been told "they are similar", and later the reality has turned out to be
"they are similar, except...". Furthermore, even if these devices seem
similar to software (which is what the comment in the MFD driver is
referring to), it does not mean these devices are 100% electrically
compatible so that they could be used as a "drop-in" replacement to each
others. I wouldn't guarantee that.
Furthermore, my current understanding is that the BD71828 was a model
that was used for a limited purposes. So, maybe creating an dt-entry like:
compatible = "rohm,bd71879", "rohm,bd71828"
might not prove to be too useful. (But I'm not 100% certain on this).
> Yes. If there are reasons against, please briefly mention them in commit
> msg.
I would like to understand the rationale for allowing:
compatible = "rohm,bd71879", "rohm,bd71828".
Is the intention to:
1) allow boards which tell the software that "the hardware may be
bd71828 or bd71879", or
2) to tell a binding reader that these ICs are likely to be usable as
replacements to each others?
(Or, is there some other rationale beyond these?)
If it's 1), then I see limited sense in doing so, while I expect that
not so many bd71828 variants will be seen out there - and at least not
in that many different products. If it's the 2), then I wouldn't say we
have the facts to do this.
And, as always, if there is 3), 4), ... - I am keen to learn :)
Yours,
-- Matti
--
Matti Vaittinen
Linux kernel developer at ROHM Semiconductors
Oulu Finland
~~ When things go utterly wrong vim users can always type :help! ~~
^ permalink raw reply
* Re: [PATCH v5 5/5] PCI: dwc: Add common send PME_Turn_Off message method
From: Manivannan Sadhasivam @ 2024-04-05 6:24 UTC (permalink / raw)
To: Frank Li
Cc: Bjorn Helgaas, Jingoo Han, Gustavo Pimentel, Lorenzo Pieralisi,
Krzysztof Wilczyński, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, imx, linux-pci, linux-kernel, devicetree
In-Reply-To: <20240319-pme_msg-v5-5-af9ffe57f432@nxp.com>
On Tue, Mar 19, 2024 at 12:07:15PM -0400, Frank Li wrote:
PCI: dwc: Add generic MSG TLP support for sending PME_Turn_Off during system suspend
> Reserve space at end of first IORESOURCE_MEM window as message TLP MMIO
> window. This space's size is 'region_align'.
>
> Set outbound ATU map memory write to send PCI message. So one MMIO write
> can trigger a PCI message, such as PME_Turn_Off.
>
> Add common dwc_pme_turn_off() function.
>
> Call dwc_pme_turn_off() to send out PME_Turn_Off message in general
> dw_pcie_suspend_noirq() if there are not platform callback pme_turn_off()
> exist.
>
How about:
"Instead of relying on the vendor specific implementations to send the
PME_Turn_Off message, let's introduce a generic way of sending the message using
the MSG TLP.
This is achieved by reserving a region for MSG TLP of size 'pci->region_align',
at the end of the first IORESOURCE_MEM window of the host bridge. And then
sending the PME_Turn_Off message during system suspend with the help of iATU.
It should be noted that this generic implementation is optional for the glue
drivers and can be overridden by a custom 'pme_turn_off' callback."
> Signed-off-by: Frank Li <Frank.Li@nxp.com>
> ---
> drivers/pci/controller/dwc/pcie-designware-host.c | 94 ++++++++++++++++++++++-
> drivers/pci/controller/dwc/pcie-designware.h | 3 +
> 2 files changed, 93 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/pci/controller/dwc/pcie-designware-host.c b/drivers/pci/controller/dwc/pcie-designware-host.c
> index 267687ab33cbc..d5723fce7a894 100644
> --- a/drivers/pci/controller/dwc/pcie-designware-host.c
> +++ b/drivers/pci/controller/dwc/pcie-designware-host.c
> @@ -393,6 +393,31 @@ static int dw_pcie_msi_host_init(struct dw_pcie_rp *pp)
> return 0;
> }
>
> +static void dw_pcie_host_request_msg_tlp_res(struct dw_pcie_rp *pp)
> +{
> + struct dw_pcie *pci = to_dw_pcie_from_pp(pp);
> + struct resource_entry *win;
> + struct resource *res;
> +
> + win = resource_list_first_type(&pp->bridge->windows, IORESOURCE_MEM);
> + if (win) {
> + res = devm_kzalloc(pci->dev, sizeof(*res), GFP_KERNEL);
> + if (!res)
> + return;
> +
> + /* Reserve last region_align block as message TLP space */
> + res->start = win->res->end - pci->region_align + 1;
> + res->end = win->res->end;
Don't you need to adjust the host bridge window size and end address?
> + res->name = "msg";
> + res->flags = win->res->flags | IORESOURCE_BUSY;
> +
Shouldn't this resource be added back to the host bridge?
> + if (!request_resource(win->res, res))
Why can't you use devm_ helper to manage the resource, since the lifetime of the
resource is till dw_pcie_host_deinit()?
> + pp->msg_res = res;
> + else
> + devm_kfree(pci->dev, res);
> + }
> +}
> +
> int dw_pcie_host_init(struct dw_pcie_rp *pp)
> {
> struct dw_pcie *pci = to_dw_pcie_from_pp(pp);
> @@ -479,6 +504,10 @@ int dw_pcie_host_init(struct dw_pcie_rp *pp)
>
> dw_pcie_iatu_detect(pci);
>
> + /* Need call after dw_pcie_iatu_detect() before dw_pcie_setup_rc() */
It'd be better to add the reason also i.,e
/*
* Allocate the resource for MSG TLP before programming the iATU
* outbound window in dw_pcie_setup_rc(). Since the allocation depends
* on the value of 'region_align', this has to be done after
* dw_pcie_iatu_detect().
*/
> + if (pp->use_atu_msg)
Who is setting this flag?
> + dw_pcie_host_request_msg_tlp_res(pp);
> +
> ret = dw_pcie_edma_detect(pci);
> if (ret)
> goto err_free_msi;
> @@ -536,6 +565,11 @@ void dw_pcie_host_deinit(struct dw_pcie_rp *pp)
>
> dw_pcie_edma_remove(pci);
>
> + if (pp->msg_res) {
> + release_resource(pp->msg_res);
> + devm_kfree(pci->dev, pp->msg_res);
> + }
> +
> if (pp->has_msi_ctrl)
> dw_pcie_free_msi(pp);
>
> @@ -697,6 +731,10 @@ static int dw_pcie_iatu_setup(struct dw_pcie_rp *pp)
> atu.pci_addr = entry->res->start - entry->offset;
> atu.size = resource_size(entry->res);
>
> + /* MSG TLB window resource reserve at one of end of IORESOURCE_MEM resource */
> + if (pp->msg_res && pp->msg_res->parent == entry->res)
> + atu.size -= resource_size(pp->msg_res);
If you adjust the bridge window above, then this won't be needed.
> +
> ret = dw_pcie_prog_outbound_atu(pci, &atu);
> if (ret) {
> dev_err(pci->dev, "Failed to set MEM range %pr\n",
> @@ -728,6 +766,8 @@ static int dw_pcie_iatu_setup(struct dw_pcie_rp *pp)
> dev_warn(pci->dev, "Ranges exceed outbound iATU size (%d)\n",
> pci->num_ob_windows);
>
> + pp->msg_atu_index = i;
> +
> i = 0;
> resource_list_for_each_entry(entry, &pp->bridge->dma_ranges) {
> if (resource_type(entry->res) != IORESOURCE_MEM)
> @@ -833,11 +873,54 @@ int dw_pcie_setup_rc(struct dw_pcie_rp *pp)
> }
> EXPORT_SYMBOL_GPL(dw_pcie_setup_rc);
>
> +/* Using message outbound ATU to send out PME_Turn_Off message for dwc PCIe controller */
> +static int dw_pcie_pme_turn_off(struct dw_pcie *pci)
> +{
> + struct dw_pcie_ob_atu_cfg atu = { 0 };
> + void __iomem *m;
*mem
> + int ret;
> +
> + if (pci->num_ob_windows <= pci->pp.msg_atu_index)
> + return -EINVAL;
> +
> + if (!pci->pp.msg_res)
> + return -EINVAL;
> +
> + atu.code = PCIE_MSG_CODE_PME_TURN_OFF;
> + atu.routing = PCIE_MSG_TYPE_R_BC;
> + atu.type = PCIE_ATU_TYPE_MSG;
> + atu.size = resource_size(pci->pp.msg_res);
> + atu.index = pci->pp.msg_atu_index;
> +
> + if (!atu.size) {
> + dev_dbg(pci->dev,
> + "atu memory map windows is zero, please check 'msg' reg in dts\n");
You are already checking the existence of the 'pci->pp.msg_res' region above. So
shouldn't that be sufficient enough? Can the size be 0, if the region exist?
- Mani
> + return -ENOMEM;
> + }
> +
> + atu.cpu_addr = pci->pp.msg_res->start;
> +
> + ret = dw_pcie_prog_outbound_atu(pci, &atu);
> + if (ret)
> + return ret;
> +
> + m = ioremap(atu.cpu_addr, pci->region_align);
> + if (!m)
> + return -ENOMEM;
> +
> + /* A dummy write is converted to a Msg TLP */
> + writel(0, m);
> +
> + iounmap(m);
> +
> + return 0;
> +}
> +
> int dw_pcie_suspend_noirq(struct dw_pcie *pci)
> {
> u8 offset = dw_pcie_find_capability(pci, PCI_CAP_ID_EXP);
> u32 val;
> - int ret;
> + int ret = 0;
>
> /*
> * If L1SS is supported, then do not put the link into L2 as some
> @@ -849,10 +932,13 @@ int dw_pcie_suspend_noirq(struct dw_pcie *pci)
> if (dw_pcie_get_ltssm(pci) <= DW_PCIE_LTSSM_DETECT_ACT)
> return 0;
>
> - if (!pci->pp.ops->pme_turn_off)
> - return 0;
> + if (pci->pp.ops->pme_turn_off)
> + pci->pp.ops->pme_turn_off(&pci->pp);
> + else
> + ret = dw_pcie_pme_turn_off(pci);
>
> - pci->pp.ops->pme_turn_off(&pci->pp);
> + if (ret)
> + return ret;
>
> ret = read_poll_timeout(dw_pcie_get_ltssm, val, val == DW_PCIE_LTSSM_L2_IDLE,
> PCIE_PME_TO_L2_TIMEOUT_US/10,
> diff --git a/drivers/pci/controller/dwc/pcie-designware.h b/drivers/pci/controller/dwc/pcie-designware.h
> index 703b50bc5e0f1..dca5de4c6e877 100644
> --- a/drivers/pci/controller/dwc/pcie-designware.h
> +++ b/drivers/pci/controller/dwc/pcie-designware.h
> @@ -341,6 +341,9 @@ struct dw_pcie_rp {
> struct pci_host_bridge *bridge;
> raw_spinlock_t lock;
> DECLARE_BITMAP(msi_irq_in_use, MAX_MSI_IRQS);
> + bool use_atu_msg;
> + int msg_atu_index;
> + struct resource *msg_res;
> };
>
> struct dw_pcie_ep_ops {
>
> --
> 2.34.1
>
--
மணிவண்ணன் சதாசிவம்
^ permalink raw reply
* Re: [PATCH 4/5] clk: qcom: Add camera clock controller driver for SM8150
From: Satya Priya Kakitapalli (Temp) @ 2024-04-05 6:27 UTC (permalink / raw)
To: Bryan O'Donoghue, Bjorn Andersson, Konrad Dybcio,
Michael Turquette, Stephen Boyd, Abhishek Sahu, Rob Herring,
Krzysztof Kozlowski, Conor Dooley
Cc: Stephen Boyd, linux-arm-msm, linux-clk, linux-kernel, devicetree,
Ajit Pandey, Imran Shaik, Taniya Das, Jagadeesh Kona
In-Reply-To: <18567989-fb60-49ae-92e6-94e1bc2fa1c7@linaro.org>
On 3/2/2024 9:43 PM, Bryan O'Donoghue wrote:
> On 29/02/2024 5:38 a.m., Satya Priya Kakitapalli wrote:
>> Add support for the camera clock controller for camera clients
>> to be able to request for camcc clocks on SM8150 platform.
>>
>> Signed-off-by: Satya Priya Kakitapalli <quic_skakitap@quicinc.com>
>> ---
>
>> +static int cam_cc_sm8150_probe(struct platform_device *pdev)
>> +{
>> + struct regmap *regmap;
>> + int ret;
>> +
>> + ret = devm_pm_runtime_enable(&pdev->dev);
>> + if (ret)
>> + return ret;
>> +
>> + ret = pm_runtime_resume_and_get(&pdev->dev);
>> + if (ret)
>> + return ret;
>> +
>> + regmap = qcom_cc_map(pdev, &cam_cc_sm8150_desc);
>> + if (IS_ERR(regmap)) {
>> + pm_runtime_put(&pdev->dev);
>> + return PTR_ERR(regmap);
>> + }
>> +
>> + clk_trion_pll_configure(&cam_cc_pll0, regmap, &cam_cc_pll0_config);
>> + clk_trion_pll_configure(&cam_cc_pll1, regmap, &cam_cc_pll1_config);
>> + clk_regera_pll_configure(&cam_cc_pll2, regmap,
>> &cam_cc_pll2_config);
>> + clk_trion_pll_configure(&cam_cc_pll3, regmap, &cam_cc_pll3_config);
>> + clk_trion_pll_configure(&cam_cc_pll4, regmap, &cam_cc_pll4_config);
>> +
>> + /* Keep the critical clock always-on */
>> + qcom_branch_set_clk_en(regmap, 0xc1e4); /* cam_cc_gdsc_clk */
>
> Does this clock need to be specified this way ?
>
> drivers/clk/qcom/camcc-sc8280xp.c::camcc_gdsc_clk specifies the gdsc
> clock as a shared op clock.
>
> Actually it looks to be register compatible, please try defining
> titan_top_gdsc as per the example in 8280xp.
>
>> +
>> + ret = qcom_cc_really_probe(pdev, &cam_cc_sm8150_desc, regmap);
>> +
>> + pm_runtime_put(&pdev->dev);
>> +
>> + return ret;
>> +}
>
> So this is a pattern we keep repeating in the clock probe() functions
> which I am writing a series to address. There's no need to continue to
> replicate the bug in new code though.
>
> Only switch on always-on clocks if probe succeeds.
>
> ret = qcom_cc_really_probe(pdev, &cam_cc_sm8150_desc, regmap);
> if (ret)
> goto probe_err;
>
> qcom_branch_set_clk_en(regmap, 0xc1e4); /* cam_cc_gdsc_clk */
>
> pm_runtime_put(&pdev->dev);
>
> return 0;
>
> probe_err:
> pm_runtime_put_sync(&pdev->dev);
>
> Alternatively switch on the always-on clocks before the really_probe()
> but then roll back in a probe_err: goto
>
> probe_err:
> remap_bits_update(regmap, 0xc1e4, BIT(0), 0);
> pm_runtime_put_sync(&pdev->dev);
>
> There may be corner cases where always-on has to happen before
> really_probe() I suppose but as a general pattern the above should be
> how we go.
>
I have rechecked this and see that this clock is PoR ON (i.e BIT(0) is
set upon power ON) and it should be kept always ON as per HW
recommendation. So even if the probe fails we shouldn't be clearing it
against the hw recommendation. We are setting the bit here again to make
sure it is set when the driver probes.
> Anyway I suspect the right thing to do is to define a
> titan_top_gdsc_clk with shared ops to "park" the GDSC clock to 19.2
> MHz instead of turning it off.
>
> You can get rid of the hard-coded always-on and indeed represent the
> clock in /sysfs - which is preferable IMO to just whacking registers
> to keep clocks always-on in probe anyway.
>
> Please try to define the titan_top_gdsc_clk as a shared_ops clock
> instead of hard coding to always on.
>
> If that doesn't work for some reason, then please fix your always-on
> logic in probe() to only make the clock fixed on, if really_probe()
> succeeds.
>
> ---
> bod
^ permalink raw reply
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