mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
From: Krishna Chaitanya Chundru <krishna.chundru@oss.qualcomm.com>
To: Alexey Charkov <alchark@flipper.net>
Cc: Manivannan Sadhasivam <mani@kernel.org>,
	Rob Herring <robh@kernel.org>,
	Krzysztof Kozlowski <krzk+dt@kernel.org>,
	Conor Dooley <conor+dt@kernel.org>,
	Bartosz Golaszewski <brgl@kernel.org>,
	Bjorn Andersson <andersson@kernel.org>,
	Konrad Dybcio <konradybcio@kernel.org>,
	linux-pci@vger.kernel.org, devicetree@vger.kernel.org,
	linux-kernel@vger.kernel.org, linux-pm@vger.kernel.org,
	linux-arm-msm@vger.kernel.org
Subject: Re: [PATCH 1/3] dt-bindings: connector: Add PCIe M.2 Mechanical Key B connector
Date: Sat, 14 Nov 2026 07:15:05 +0530	[thread overview]
Message-ID: <861b48b3-32e2-459a-a6e1-75d93f10e1c9@oss.qualcomm.com> (raw)
In-Reply-To: <CAKTNdwEzbYAqM5ABxU+tfh4juU=wgYp=QXyjvdA1haanU0e8tQ@mail.gmail.com>



On 10/22/2026 2:56 PM, Alexey Charkov wrote:
> On Thu, Oct 22, 2026 at 8:27 AM Krishna Chaitanya Chundru
> <krishna.chundru@oss.qualcomm.com> wrote:
>>
>>
>> On 10/21/2026 9:14 PM, Alexey Charkov wrote:
>>> Hi Krishna,
>> Hi Alexey,
>>
>> Thanks for the thorough review, this is helpful.
>>> On Wed, Oct 21, 2026 at 5:51 PM Krishna Chaitanya Chundru
>>> <krishna.chundru@oss.qualcomm.com> wrote:
>>>> Add the devicetree binding for PCIe M.2 Mechanical Key B connector
>>>> defined in the PCI Express M.2 Specification, r4.2, sec 3.2.19.1. This
>>>> connector is used to attach WWAN Adapters to the host machine over
>>>> interfaces such as PCIe, USB3.1 Gen1, SSIC, or USB 2.0, selected by the
>>>> Adapter's configuration pins.
>>>>
>>>> The connector provides a primary power supply of 3.3V/VBAT, along with
>>>> optional GPIO signals for WWAN-specific power/radio control:
>>>> FULL_CARD_POWER_OFF#, RESET#, W_DISABLE1#/2#, WAKE_OQN_WWAN#, and DPR.
>>>>
>>>> Signed-off-by: Krishna Chaitanya Chundru <krishna.chundru@oss.qualcomm.com>
>>>> ---
>>>>  .../bindings/connector/pcie-m2-b-connector.yaml    | 146 +++++++++++++++++++++
>>>>  MAINTAINERS                                        |   1 +
>>>>  2 files changed, 147 insertions(+)
>>>>
>>>> diff --git a/Documentation/devicetree/bindings/connector/pcie-m2-b-connector.yaml b/Documentation/devicetree/bindings/connector/pcie-m2-b-connector.yaml
>>>> new file mode 100644
>>>> index 000000000000..a7f4bcbc5119
>>>> --- /dev/null
>>>> +++ b/Documentation/devicetree/bindings/connector/pcie-m2-b-connector.yaml
>>>> @@ -0,0 +1,146 @@
>>>> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
>>>> +%YAML 1.2
>>>> +---
>>>> +$id: http://devicetree.org/schemas/connector/pcie-m2-b-connector.yaml#
>>>> +$schema: http://devicetree.org/meta-schemas/core.yaml#
>>>> +
>>>> +title: PCIe M.2 Mechanical Key B Connector
>>>> +
>>>> +maintainers:
>>>> +  - Krishna Chaitanya Chundru <krishna.chundru@oss.qualcomm.com>
>>>> +
>>>> +description:
>>>> +  A PCIe M.2 B connector node represents a physical PCIe M.2 Mechanical Key B
>>>> +  connector. Mechanical Key B connectors are used to connect WWAN devices to
>>>> +  the host machine over interfaces like PCIe, USB3.1 Gen1, SSIC, or USB 2.0,
>>>> +  selected by the Adapter's configuration pins.
>>>> +
>>>> +properties:
>>>> +  compatible:
>>>> +    const: pcie-m2-b-connector
>>>> +
>>>> +  vpcie3v3-supply:
>>>> +    description: A phandle to the regulator for the 3.3v/VBAT supply.
>>>> +
>>>> +  full-card-power-off-gpios:
>>>> +    description: GPIO output to FULL_CARD_POWER_OFF# signal. This signal is
>>>> +      used by the host system to turn off the entire Adapter. Refer, PCI
>>>> +      Express M.2 Specification r4.0, sec 3.2.13.1 for more details.
>>>> +    maxItems: 1
>>>> +
>>>> +  reset-gpios:
>>>> +    description: GPIO output to RESET# signal. This signal is used by the
>>>> +      host system to place the WWAN Adapter into a power-on reset condition.
>>>> +      Refer, PCI Express M.2 Specification r4.0, sec 3.2.13.2 for more
>>>> +      details.
>>>> +    maxItems: 1
>>>> +
>>>> +  w-disable1-gpios:
>>>> +    description: GPIO output to W_DISABLE1# signal. This signal is used by
>>>> +      the host system to disable the WWAN radio on the Adapter. Refer, PCI
>>>> +      Express M.2 Specification r4.0, sec 3.1.12.3 for more details.
>>>> +    maxItems: 1
>>>> +
>>>> +  w-disable2-gpios:
>>>> +    description: GPIO output to W_DISABLE2# signal (default function of
>>>> +      GPIO_10). This signal is used by the host system to disable a second
>>>> +      radio on the Adapter. Refer, PCI Express M.2 Specification r4.0,
>>>> +      sec 3.1.12.3 for more details.
>>>> +    maxItems: 1
>>>> +
>>>> +  wowwan-gpios:
>>>> +    description: GPIO input to WAKE_ON_WWAN# (WoWWAN#) signal. The Adapter
>>>> +      asserts this signal to wake up the host system. Refer, PCI Express M.2
>>>> +      Specification r4.0, sec 3.2.13.3.6 for more details.
>>>> +    maxItems: 1
>>>> +
>>>> +  dpr-gpios:
>>>> +    description: GPIO output to the Dynamic Power Reduction (DPR) signal.
>>>> +      This signal is provided by a host system proximity sensor to the
>>>> +      Adapter to trigger a reduction in the radio transmit output power to
>>>> +      meet regulatory SAR requirements. Refer, PCI Express M.2 Specification
>>>> +      r4.0, sec 3.2.13.4 for more details.
>>>> +    maxItems: 1
>>> This seems to be missing the four CONFIG_* pins, which are quite
>>> crucial in these connectors, as they define which interfaces the
>>> module asks the host to provide on the respective pins. Without those
>>> module type detection is impossible.
>> Agreed, thanks for catching this. I will add a config-gpios property  (4 GPIO
>> inputs, one per CONFIG_0..3 pin) to v2. It's optional, not required - a board
>> that always has a known, fixed card type in a given socket (e.g. QCOM
>> glymur-crd's WWAN-only slot) can just describe that fixed wiring via the ports
>> property and skip CONFIG decode entirely; config-gpios is only needed when the
>> host wants to detect the card type/host-interface at runtime. Kept it as plain
>> sensed GPIOs with no decode semantics imposed by the schema - the property
>> description documents the full Table 3-18 state table inline (all 16 states)
>> so the binding is self-contained, but turning the sensed 4-bit pattern into
>> "this is a WWAN-PCIe Adapter in port config 0" is left to driver code,
>> consistent with how the rest of the connector's sensed straps (e.g.
>> viocfg-gpios on the E-connector) are already handled.
> Agreed that the logic of turning the 4-bit pattern into
> enabling/disabling interfaces sounds like a driver realm, although we
> probably need to consider if the connector should accept pinctrl-0
> through pinctrl-15 to hint the host about which pin muxes should
> follow from each of the 16 4-bit CONFIG_* pin states. This mapping is
> hard to recover outside the connector's context.
 I'd put the pinctrl states on the connector, one per CONFIG_0..3 value (Table
3-18), selected after the driver reads config-gpios. The CONFIG-to-mux mapping
can't be recovered from the interface providers, as you said. Providers would
keep only their own default pinctrl. What I'm unsure about is how much is
SoC-muxed in practice. The high-speed pairs are often fixed by PHY or lane
routing, so the states may mostly matter for GPIO_0..11. Do you know of a case
where they cover more than that?
>>>> +  clocks:
>>>> +    description: 32.768 KHz Suspend Clock (SUSCLK) input from the host system to
>>>> +      the M.2 card. Refer, PCI Express M.2 Specification r4.0, sec 3.1.12.1 for
>>>> +      more details.
>>>> +    maxItems: 1
>>>> +
>>>> +  ports:
>>>> +    $ref: /schemas/graph.yaml#/properties/ports
>>>> +    description: OF graph bindings modeling the interfaces exposed on the
>>>> +      connector. Since a single connector can have multiple interfaces, every
>>>> +      interface has an assigned OF graph port number as described below.
>>>> +
>>>> +    properties:
>>>> +      port@0:
>>>> +        $ref: /schemas/graph.yaml#/properties/port
>>>> +        description: PCIe interface for WWAN
>>>> +
>>>> +      port@1:
>>>> +        $ref: /schemas/graph.yaml#/properties/port
>>>> +        description: SSIC interface for WWAN
>>>> +
>>>> +      port@2:
>>>> +        $ref: /schemas/graph.yaml#/properties/port
>>>> +        description: USB 2.0 interface for WWAN
>>>> +
>>>> +      port@3:
>>>> +        $ref: /schemas/graph.yaml#/properties/port
>>>> +        description: USB3.1 Gen1 interface for WWAN
>>>> +
>>>> +    anyOf:
>>>> +      - required:
>>>> +          - port@0
>>>> +      - required:
>>>> +          - port@1
>>>> +      - required:
>>>> +          - port@3
>>> This misses SATA which shares pins with PCIe. PCIe second lane also
>>> conflicts with USB 3.1/SSIC, which isn't captured here. Should it be?
>> Before adding a SATA/SSD port here, I want to double check the scope: per
>> Table 2-1, every SSD-capable board using those CONFIG states is a B+M dual-key
>> card (Type 2230/2242/2260/2280/22110, "B+M" key column), not a pure Key-B-only
>> card. Pure Key-B-only form factors (2242, 3042, 3052, 3060) are WWAN-only per
>> that table, with no SSD row.
> Your binding is about the host-side M.2 key B slot, which by design
> accepts both M.2 key B cards and M.2 key B+M cards, and the slot
> interfaces are explicitly designed to support both. So the binding
> should reflect the slot's hardware, not the card's hardware.
>
>> If that holds, is modeling SATA/PCIe-SSD states in pcie-m2-b-connector even in
>> scope, versus always describing a B+M socket via pcie-m2-m-connector (which
>> already covers PCIe/SATA SSDs) and reserving pcie-m2-b-connector strictly for
>> the WWAN-only Key-B form factors? Or do you know of a real case where a pure
>> Key-B socket legitimately decodes to an SSD CONFIG state that I'm missing?
>> Happy to add the SATA port either way, just want to scope it correctly before v2.
> I don't think B+M _sockets_ even exist. The socket is keyed either B,
> or M, not both, and a B+M keyed card is compatible with either. It
> also reflects real-world use: people can and do use SATA SSDs in key-B
> slots. Same with NVMe SSDs, although those are rarer in B+M form
> factor (but I own one, and it works fine in a full-function
> spec-compliant key-B slot).
>
>>> Should the graph also envisage connections for the I2C bus (GNSS),
>>> audio (2 or 4 wire), re-muxing of W_DISABLE2# and WAKE_ON_WWAN# into
>>> HSIC functions in Port Config_3, GNSS interrupt line and the such?
>>> Should the connector node also define pinctrl configurations for each
>>> of the 15 interface configurations?
>> These are all real signals in the spec (GPIO_0-11's BTO-defined functions per
>> Table 3-15, plus the HSIC remux under Port Config_3), but I'd left them out
>> deliberately for now: every one of GPIO_0 through GPIO_9 is explicitly "VENDOR
>> DEFINED or GPIO_x" in the spec text, i.e. there's no fixed function to bind to
>> without a BTO agreement between card vendor and platform. The sibling
>> pcie-m2-e-connector binding takes the same approach and doesn't model its own
>> analogous BTO pins either.
> I thought only configuration b1011 explicitly says that the other
> interfaces besides PCIe and USB 3.1 Gen 1 are vendor defined. The rest
> have an explicitly assigned function for GPIO_0..GPIO_11, as set out
> in the Port Config_0 through Port Config_3 mapping table. Port
> Config_3 is the weird one, with its vendor-defined IPC on most of the
> pins, but the others are more clear-cut.
>
>> I'd rather land CONFIG_0-3 decode plus the SATA/PCIe-SSD scoping question in
>> v2, and handle I2C/GNSS/audio/HSIC-remux/per-config pinctrl as follow-up
>> patches once there's a concrete board that needs one of them with a real
>> driver consumer - adding properties nothing reads yet felt like the wrong
>> order of operations. But let me know if you think v1 shouldn't go in at all
>> until that full state machine is captured in the binding.
> The general guidance is that bindings should be complete, even if the
> software does not fully implement them yet. I'm not sure how strict
> that guidance is.
>
> In this particular case I'm wary of the risk in merging a version of
> the binding which will make sane modeling of those extra interfaces
> impossible later on. Assigning port nodes to all possible interfaces
> envisaged by the spec is probably easy enough, although subtleties
> remain, such as: do pinctrl groups for each interface configuration
> belong to the respective interface providers or to the connector? Does
> the binding allow additional vendor-specific interfaces on the
> GPIO_0..GPIO_11 pins at all (e.g. various IPC configurations which the
> spec leaves to the host and module vendor to decide), and if yes -
> how?
>
> These decisions (and probably others I haven't thought of yet) will
> likely shape the correct binding structure. It would be great to
> discuss and prototype them before committing to the binding, which
> must remain backward-compatible once merged.
>
Vendor-defined interfaces: only Port Config_3's IPC_0..7 and the
vendor-defined state 14 are open-ended. I'm undecided. One option is to expose
those pins as plain GPIOs on the connector, so a board can use them without a
binding revision. The other is to leave them out until a board needs them.
Which would you prefer?

Ack, for remaining comments.

- Krishna Chaitanya.
> Best regards,
> Alexey


  reply	other threads:[~2026-11-14  1:45 UTC|newest]

Thread overview: 11+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-10-21 13:51 [PATCH 0/3] power: sequencing: pcie-m2: Add PCIe M.2 Key B connector support Krishna Chaitanya Chundru
2026-10-21 13:51 ` [PATCH 1/3] dt-bindings: connector: Add PCIe M.2 Mechanical Key B connector Krishna Chaitanya Chundru
2026-10-21 15:44   ` Alexey Charkov
2026-10-22  4:26     ` Krishna Chaitanya Chundru
2026-10-22  9:26       ` Alexey Charkov
2026-11-14  1:45         ` Krishna Chaitanya Chundru [this message]
2026-10-30 17:44   ` Rob Herring
2026-11-14  1:45     ` Krishna Chaitanya Chundru
2026-10-21 13:51 ` [PATCH 2/3] power: sequencing: pcie-m2: Add support for PCIe M.2 Key B connectors Krishna Chaitanya Chundru
2026-10-21 13:51 ` [PATCH 3/3] arm64: dts: qcom: glymur-crd: Describe the PCIe M.2 Key B connector Krishna Chaitanya Chundru
2026-10-21 14:06   ` Konrad Dybcio

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=861b48b3-32e2-459a-a6e1-75d93f10e1c9@oss.qualcomm.com \
    --to=krishna.chundru@oss.qualcomm.com \
    --cc=alchark@flipper.net \
    --cc=andersson@kernel.org \
    --cc=brgl@kernel.org \
    --cc=conor+dt@kernel.org \
    --cc=devicetree@vger.kernel.org \
    --cc=konradybcio@kernel.org \
    --cc=krzk+dt@kernel.org \
    --cc=linux-arm-msm@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-pci@vger.kernel.org \
    --cc=linux-pm@vger.kernel.org \
    --cc=mani@kernel.org \
    --cc=robh@kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

all inboxes | Powered by JetHome®