* Re: [PATCH v5 02/10] dt-bindings: mailbox: Add mboxes property for CMDQ secure driver
From: Rob Herring @ 2024-04-03 11:43 UTC (permalink / raw)
To: Shawn Sung
Cc: CK Hu, Krzysztof Kozlowski, linux-kernel, linux-arm-kernel,
Conor Dooley, Jason-JH . Lin, AngeloGioacchino Del Regno,
Houlong Wei, Jassi Brar, devicetree, linux-mediatek,
Matthias Brugger
In-Reply-To: <20240403102602.32155-3-shawn.sung@mediatek.com>
On Wed, 03 Apr 2024 18:25:54 +0800, Shawn Sung wrote:
> From: "Jason-JH.Lin" <jason-jh.lin@mediatek.com>
>
> Add mboxes to define a GCE loopping thread as a secure irq handler.
> This property is only required if CMDQ secure driver is supported.
>
> Signed-off-by: Jason-JH.Lin <jason-jh.lin@mediatek.com>
> Signed-off-by: Hsiao Chien Sung <shawn.sung@mediatek.com>
> ---
> .../bindings/mailbox/mediatek,gce-mailbox.yaml | 10 ++++++++++
> 1 file changed, 10 insertions(+)
>
My bot found errors running 'make DT_CHECKER_FLAGS=-m dt_binding_check'
on your patch (DT_CHECKER_FLAGS is new in v5.13):
yamllint warnings/errors:
dtschema/dtc warnings/errors:
/builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/mailbox/mediatek,gce-mailbox.yaml:
Unresolvable JSON pointer: 'definitions/uint32-arrayi'
doc reference errors (make refcheckdocs):
See https://patchwork.ozlabs.org/project/devicetree-bindings/patch/20240403102602.32155-3-shawn.sung@mediatek.com
The base for the series is generally the latest rc1. A different dependency
should be noted in *this* patch.
If you already ran 'make dt_binding_check' and didn't see the above
error(s), then make sure 'yamllint' is installed and dt-schema is up to
date:
pip3 install dtschema --upgrade
Please check and re-submit after running the above command yourself. Note
that DT_SCHEMA_FILES can be set to your schema file to speed up checking
your schema. However, it must be unset to test all examples with your schema.
^ permalink raw reply
* Re: [PATCH v2 0/4] HID: Add support for Himax HX83102j touchscreen
From: Jiri Kosina @ 2024-04-03 11:48 UTC (permalink / raw)
To: Allen_Lin
Cc: dmitry.torokhov, robh, krzysztof.kozlowski+dt, conor,
benjamin.tissoires, linux-input, devicetree, linux-kernel
In-Reply-To: <TY0PR06MB561132DF147C037093A1B94D9E3E2@TY0PR06MB5611.apcprd06.prod.outlook.com>
On Tue, 2 Apr 2024, Allen_Lin wrote:
> Hi,
> This driver implements for Himax HID touchscreen HX83102j.
>
> Using SPI interface to receive/send HID packets.
>
> Changes in v2 :
> -Added power description in YAML document.
> -Added ddreset-gpios property in YAML document.
> -Added firmware-name property in YAML document.
> -Modified the description of pid.
> -Modified the example.
>
> Allen_Lin (4):
> dt-bindings: input: Add Himax HX83102J touchscreen
> HID: Add Himax HX83102J touchscreen driver
> HID: Add DRM panel follower function
> HID: Load firmware directly from file to IC
>
> .../input/touchscreen/himax,hx83102j.yaml | 100 +
> MAINTAINERS | 7 +
> drivers/hid/Kconfig | 7 +
> drivers/hid/Makefile | 2 +
> drivers/hid/hid-himax-83102j.c | 3071 +++++++++++++++++
> drivers/hid/hid-himax-83102j.h | 460 +++
My only nit here -- could we please call the driver just hid-himax, to
follow the pattern we generally use in this subsystem (drivers named after
vendors).
Please add Ack from Rob, rename the driver, resend, and I'll apply it.
Thanks,
--
Jiri Kosina
SUSE Labs
^ permalink raw reply
* Re: [PATCH v2 16/18] PCI: rockchip-ep: Improve link training
From: Rick Wertenbroek @ 2024-04-03 11:54 UTC (permalink / raw)
To: Damien Le Moal
Cc: Manivannan Sadhasivam, Lorenzo Pieralisi, Kishon Vijay Abraham I,
Shawn Lin, Krzysztof Wilczyński, Bjorn Helgaas,
Heiko Stuebner, linux-pci, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, devicetree, linux-rockchip, linux-arm-kernel,
Wilfred Mallawa, Niklas Cassel
In-Reply-To: <20240330041928.1555578-17-dlemoal@kernel.org>
On Sat, Mar 30, 2024 at 5:20 AM Damien Le Moal <dlemoal@kernel.org> wrote:
>
> The Rockchip rk339 technical reference manual describe the endpoint mode
> link training process clearly and states that:
> Insure link training completion and success by observing link_st field
> in PCIe Client BASIC_STATUS1 register change to 2'b11. If both side
> support PCIe Gen2 speed, re-train can be Initiated by asserting the
> Retrain Link field in Link Control and Status Register. The software
> should insure the BASIC_STATUS0[negotiated_speed] changes to "1", that
> indicates re-train to Gen2 successfully.
> This procedure is very similar to what is done for the root-port mode in
> rockchip_pcie_host_init_port().
>
> Implement this link training procedure for the endpoint mode as well.
> Given that the rk3399 SoC does not have an interrupt signaling link
> status changes, training is implemented as a delayed work which is
> rescheduled until the link training completes or the endpoint controller
> is stopped. The link training work is first scheduled in
> rockchip_pcie_ep_start() when the endpoint function is started. Link
> training completion is signaled to the function using pci_epc_linkup().
> Accordingly, the linkup_notifier field of the rockchip pci_epc_features
> structure is changed to true.
>
> Signed-off-by: Damien Le Moal <dlemoal@kernel.org>
> ---
> drivers/pci/controller/pcie-rockchip-ep.c | 79 ++++++++++++++++++++++-
> drivers/pci/controller/pcie-rockchip.h | 11 ++++
> 2 files changed, 89 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/pci/controller/pcie-rockchip-ep.c b/drivers/pci/controller/pcie-rockchip-ep.c
> index 2767e8f1771d..4006e7dee71a 100644
> --- a/drivers/pci/controller/pcie-rockchip-ep.c
> +++ b/drivers/pci/controller/pcie-rockchip-ep.c
> @@ -16,6 +16,8 @@
> #include <linux/platform_device.h>
> #include <linux/pci-epf.h>
> #include <linux/sizes.h>
> +#include <linux/workqueue.h>
> +#include <linux/iopoll.h>
>
> #include "pcie-rockchip.h"
>
> @@ -48,6 +50,7 @@ struct rockchip_pcie_ep {
> u64 irq_pci_addr;
> u8 irq_pci_fn;
> u8 irq_pending;
> + struct delayed_work link_training;
> };
>
> static void rockchip_pcie_clear_ep_ob_atu(struct rockchip_pcie *rockchip,
> @@ -467,6 +470,8 @@ static int rockchip_pcie_ep_start(struct pci_epc *epc)
> PCIE_CLIENT_CONF_ENABLE,
> PCIE_CLIENT_CONFIG);
>
> + schedule_delayed_work(&ep->link_training, 0);
> +
> return 0;
> }
>
> @@ -475,6 +480,8 @@ static void rockchip_pcie_ep_stop(struct pci_epc *epc)
> struct rockchip_pcie_ep *ep = epc_get_drvdata(epc);
> struct rockchip_pcie *rockchip = &ep->rockchip;
>
> + cancel_delayed_work_sync(&ep->link_training);
> +
> /* Stop link training and disable configuration */
> rockchip_pcie_write(rockchip,
> PCIE_CLIENT_CONF_DISABLE |
> @@ -482,8 +489,77 @@ static void rockchip_pcie_ep_stop(struct pci_epc *epc)
> PCIE_CLIENT_CONFIG);
> }
>
> +static void rockchip_pcie_ep_retrain_link(struct rockchip_pcie *rockchip)
> +{
> + u32 status;
> +
> + status = rockchip_pcie_read(rockchip, PCIE_EP_CONFIG_LCS);
> + status |= PCI_EXP_LNKCTL_RL;
> + rockchip_pcie_write(rockchip, status, PCIE_EP_CONFIG_LCS);
> +}
> +
> +static bool rockchip_pcie_ep_link_up(struct rockchip_pcie *rockchip)
> +{
> + u32 val = rockchip_pcie_read(rockchip, PCIE_CLIENT_BASIC_STATUS1);
> +
> + return PCIE_LINK_UP(val);
> +}
> +
> +static void rockchip_pcie_ep_link_training(struct work_struct *work)
> +{
> + struct rockchip_pcie_ep *ep =
> + container_of(work, struct rockchip_pcie_ep, link_training.work);
> + struct rockchip_pcie *rockchip = &ep->rockchip;
> + struct device *dev = rockchip->dev;
> + u32 val;
> + int ret;
> +
> + /* Enable Gen1 training and wait for its completion */
> + ret = readl_poll_timeout(rockchip->apb_base + PCIE_CORE_CTRL,
> + val, PCIE_LINK_TRAINING_DONE(val), 50,
> + LINK_TRAIN_TIMEOUT);
> + if (ret)
> + goto again;
> +
> + /* Make sure that the link is up */
> + ret = readl_poll_timeout(rockchip->apb_base + PCIE_CLIENT_BASIC_STATUS1,
> + val, PCIE_LINK_UP(val), 50,
> + LINK_TRAIN_TIMEOUT);
> + if (ret)
> + goto again;
> +
> + /* Check the current speed */
> + val = rockchip_pcie_read(rockchip, PCIE_CORE_CTRL);
> + if (!PCIE_LINK_IS_GEN2(val) && rockchip->link_gen == 2) {
> + /* Enable retrain for gen2 */
> + rockchip_pcie_ep_retrain_link(rockchip);
> + readl_poll_timeout(rockchip->apb_base + PCIE_CORE_CTRL,
> + val, PCIE_LINK_IS_GEN2(val), 50,
> + LINK_TRAIN_TIMEOUT);
> + }
> +
> + /* Check again that the link is up */
> + if (!rockchip_pcie_ep_link_up(rockchip))
> + goto again;
> +
> + val = rockchip_pcie_read(rockchip, PCIE_CLIENT_BASIC_STATUS0);
> + dev_info(dev,
> + "Link UP (Negociated speed: %sGT/s, width: x%lu)\n",
> + (val & PCIE_CLIENT_NEG_LINK_SPEED) ? "5" : "2.5",
> + ((val & PCIE_CLIENT_NEG_LINK_WIDTH_MASK) >>
> + PCIE_CLIENT_NEG_LINK_WIDTH_SHIFT) << 1);
> +
This does not print the correct link width for x1 :
# [ 60.518339] rockchip-pcie-ep fd000000.pcie-ep: Link UP
(Negociated speed: 5GT/s, width: x0)
This is because :
((val & PCIE_CLIENT_NEG_LINK_WIDTH_MASK) >>
PCIE_CLIENT_NEG_LINK_WIDTH_SHIFT) << 1
will print 0 if the link width is 1, because bits 7:6 are 0b00, and
0b00 << 1 is still 0. (0b00 => x0, 0b01 => x2, 0b10 => x4)
Therefore the formula should be :
1 << ((val & PCIE_CLIENT_NEG_LINK_WIDTH_MASK) >>
PCIE_CLIENT_NEG_LINK_WIDTH_SHIFT)
This shows the correct link width for all cases (0b00 => x1, 0b01 =>
x2, 0b10 => x4).
Reference : RK3399 TRM V1.3 pages 768-769 PCIE_CLIENT_BASIC_STATUS0
register description
> + /* Notify the function */
> + pci_epc_linkup(ep->epc);
> +
> + return;
> +
> +again:
> + schedule_delayed_work(&ep->link_training, msecs_to_jiffies(5));
> +}
> +
> static const struct pci_epc_features rockchip_pcie_epc_features = {
> - .linkup_notifier = false,
> + .linkup_notifier = true,
> .msi_capable = true,
> .msix_capable = false,
> .align = ROCKCHIP_PCIE_AT_SIZE_ALIGN,
> @@ -644,6 +720,7 @@ static int rockchip_pcie_ep_probe(struct platform_device *pdev)
> rockchip = &ep->rockchip;
> rockchip->is_rc = false;
> rockchip->dev = dev;
> + INIT_DELAYED_WORK(&ep->link_training, rockchip_pcie_ep_link_training);
>
> epc = devm_pci_epc_create(dev, &rockchip_pcie_epc_ops);
> if (IS_ERR(epc)) {
> diff --git a/drivers/pci/controller/pcie-rockchip.h b/drivers/pci/controller/pcie-rockchip.h
> index 0263f158ee8d..3963b7097a91 100644
> --- a/drivers/pci/controller/pcie-rockchip.h
> +++ b/drivers/pci/controller/pcie-rockchip.h
> @@ -26,6 +26,7 @@
> #define MAX_LANE_NUM 4
> #define MAX_REGION_LIMIT 32
> #define MIN_EP_APERTURE 28
> +#define LINK_TRAIN_TIMEOUT (5000 * USEC_PER_MSEC)
>
> #define PCIE_CLIENT_BASE 0x0
> #define PCIE_CLIENT_CONFIG (PCIE_CLIENT_BASE + 0x00)
> @@ -50,6 +51,10 @@
> #define PCIE_CLIENT_DEBUG_LTSSM_MASK GENMASK(5, 0)
> #define PCIE_CLIENT_DEBUG_LTSSM_L1 0x18
> #define PCIE_CLIENT_DEBUG_LTSSM_L2 0x19
> +#define PCIE_CLIENT_BASIC_STATUS0 (PCIE_CLIENT_BASE + 0x44)
> +#define PCIE_CLIENT_NEG_LINK_WIDTH_MASK GENMASK(7, 6)
> +#define PCIE_CLIENT_NEG_LINK_WIDTH_SHIFT 6
> +#define PCIE_CLIENT_NEG_LINK_SPEED BIT(5)
> #define PCIE_CLIENT_BASIC_STATUS1 (PCIE_CLIENT_BASE + 0x48)
> #define PCIE_CLIENT_LINK_STATUS_UP 0x00300000
> #define PCIE_CLIENT_LINK_STATUS_MASK 0x00300000
> @@ -87,6 +92,8 @@
>
> #define PCIE_CORE_CTRL_MGMT_BASE 0x900000
> #define PCIE_CORE_CTRL (PCIE_CORE_CTRL_MGMT_BASE + 0x000)
> +#define PCIE_CORE_PL_CONF_LS_MASK 0x00000001
> +#define PCIE_CORE_PL_CONF_LS_READY 0x00000001
> #define PCIE_CORE_PL_CONF_SPEED_5G 0x00000008
> #define PCIE_CORE_PL_CONF_SPEED_MASK 0x00000018
> #define PCIE_CORE_PL_CONF_LANE_MASK 0x00000006
> @@ -144,6 +151,7 @@
> #define PCIE_RC_CONFIG_BASE 0xa00000
> #define PCIE_EP_CONFIG_BASE 0xa00000
> #define PCIE_EP_CONFIG_DID_VID (PCIE_EP_CONFIG_BASE + 0x00)
> +#define PCIE_EP_CONFIG_LCS (PCIE_EP_CONFIG_BASE + 0xd0)
> #define PCIE_RC_CONFIG_RID_CCR (PCIE_RC_CONFIG_BASE + 0x08)
> #define PCIE_RC_CONFIG_DCR (PCIE_RC_CONFIG_BASE + 0xc4)
> #define PCIE_RC_CONFIG_DCR_CSPL_SHIFT 18
> @@ -155,6 +163,7 @@
> #define PCIE_RC_CONFIG_LINK_CAP (PCIE_RC_CONFIG_BASE + 0xcc)
> #define PCIE_RC_CONFIG_LINK_CAP_L0S BIT(10)
> #define PCIE_RC_CONFIG_LCS (PCIE_RC_CONFIG_BASE + 0xd0)
> +#define PCIE_EP_CONFIG_LCS (PCIE_EP_CONFIG_BASE + 0xd0)
> #define PCIE_RC_CONFIG_L1_SUBSTATE_CTRL2 (PCIE_RC_CONFIG_BASE + 0x90c)
> #define PCIE_RC_CONFIG_THP_CAP (PCIE_RC_CONFIG_BASE + 0x274)
> #define PCIE_RC_CONFIG_THP_CAP_NEXT_MASK GENMASK(31, 20)
> @@ -192,6 +201,8 @@
> #define ROCKCHIP_VENDOR_ID 0x1d87
> #define PCIE_LINK_IS_L2(x) \
> (((x) & PCIE_CLIENT_DEBUG_LTSSM_MASK) == PCIE_CLIENT_DEBUG_LTSSM_L2)
> +#define PCIE_LINK_TRAINING_DONE(x) \
> + (((x) & PCIE_CORE_PL_CONF_LS_MASK) == PCIE_CORE_PL_CONF_LS_READY)
> #define PCIE_LINK_UP(x) \
> (((x) & PCIE_CLIENT_LINK_STATUS_MASK) == PCIE_CLIENT_LINK_STATUS_UP)
> #define PCIE_LINK_IS_GEN2(x) \
> --
> 2.44.0
>
Tested-by: Rick Wertenbroek <rick.wertenbroek@gmail.com>
Best regards,
Rick
^ permalink raw reply
* Re: [PATCH v2 02/18] PCI: endpoint: Introduce pci_epc_map_align()
From: Kishon Vijay Abraham I @ 2024-04-03 12:33 UTC (permalink / raw)
To: Damien Le Moal, Manivannan Sadhasivam, Lorenzo Pieralisi,
Kishon Vijay Abraham I, Shawn Lin, Krzysztof Wilczyński,
Bjorn Helgaas, Heiko Stuebner, linux-pci, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, devicetree
Cc: linux-rockchip, linux-arm-kernel, Rick Wertenbroek,
Wilfred Mallawa, Niklas Cassel
In-Reply-To: <20240330041928.1555578-3-dlemoal@kernel.org>
Hi Damien,
On 3/30/2024 9:49 AM, Damien Le Moal wrote:
> Some endpoint controllers have requirements on the alignment of the
> controller physical memory address that must be used to map a RC PCI
> address region. For instance, the rockchip endpoint controller uses
> at most the lower 20 bits of a physical memory address region as the
> lower bits of an RC PCI address. For mapping a PCI address region of
> size bytes starting from pci_addr, the exact number of address bits
> used is the number of address bits changing in the address range
> [pci_addr..pci_addr + size - 1].
>
> For this example, this creates the following constraints:
> 1) The offset into the controller physical memory allocated for a
> mapping depends on the mapping size *and* the starting PCI address
> for the mapping.
> 2) A mapping size cannot exceed the controller windows size (1MB) minus
> the offset needed into the allocated physical memory, which can end
> up being a smaller size than the desired mapping size.
>
> Handling these constraints independently of the controller being used in
> a PCI EP function driver is not possible with the current EPC API as
> it only provides the ->align field in struct pci_epc_features.
> Furthermore, this alignment is static and does not depend on a mapping
> pci address and size.
>
> Solve this by introducing the function pci_epc_map_align() and the
> endpoint controller operation ->map_align to allow endpoint function
> drivers to obtain the size and the offset into a controller address
> region that must be used to map an RC PCI address region. The size
> of the physical address region provided by pci_epc_map_align() can then
> be used as the size argument for the function pci_epc_mem_alloc_addr().
> The offset into the allocated controller memory can be used to
> correctly handle data transfers. Of note is that pci_epc_map_align() may
> indicate upon return a mapping size that is smaller (but not 0) than the
> requested PCI address region size. For such case, an endpoint function
> driver must handle data transfers in fragments.
>
> The controller operation ->map_align is optional: controllers that do
> not have any address alignment constraints for mapping a RC PCI address
> region do not need to implement this operation. For such controllers,
> pci_epc_map_align() always returns the mapping size as equal
> to the requested size and an offset equal to 0.
>
> The structure pci_epc_map is introduced to represent a mapping start PCI
> address, size and the size and offset into the controller memory needed
> for mapping the PCI address region.
>
> Signed-off-by: Damien Le Moal <dlemoal@kernel.org>
> ---
> drivers/pci/endpoint/pci-epc-core.c | 66 +++++++++++++++++++++++++++++
> include/linux/pci-epc.h | 33 +++++++++++++++
> 2 files changed, 99 insertions(+)
>
> diff --git a/drivers/pci/endpoint/pci-epc-core.c b/drivers/pci/endpoint/pci-epc-core.c
> index 754afd115bbd..37758ca91d7f 100644
> --- a/drivers/pci/endpoint/pci-epc-core.c
> +++ b/drivers/pci/endpoint/pci-epc-core.c
> @@ -433,6 +433,72 @@ void pci_epc_unmap_addr(struct pci_epc *epc, u8 func_no, u8 vfunc_no,
> }
> EXPORT_SYMBOL_GPL(pci_epc_unmap_addr);
>
> +/**
> + * pci_epc_map_align() - Get the offset into and the size of a controller memory
> + * address region needed to map a RC PCI address region
> + * @epc: the EPC device on which address is allocated
> + * @func_no: the physical endpoint function number in the EPC device
> + * @vfunc_no: the virtual endpoint function number in the physical function
> + * @pci_addr: PCI address to which the physical address should be mapped
> + * @size: the size of the mapping starting from @pci_addr
> + * @map: populate here the actual size and offset into the controller memory
> + * that must be allocated for the mapping
> + *
> + * Invoke the controller map_align operation to obtain the size and the offset
> + * into a controller address region that must be allocated to map @size
> + * bytes of the RC PCI address space starting from @pci_addr.
> + *
> + * The size of the mapping that can be handled by the controller is indicated
> + * using the pci_size field of @map. This size may be smaller than the requested
> + * @size. In such case, the function driver must handle the mapping using
> + * several fragments. The offset into the controller memory for the effective
> + * mapping of the @pci_addr..@pci_addr+@map->pci_size address range is indicated
> + * using the map_ofst field of @map.
> + */
> +int pci_epc_map_align(struct pci_epc *epc, u8 func_no, u8 vfunc_no,
> + u64 pci_addr, size_t size, struct pci_epc_map *map)
> +{
> + const struct pci_epc_features *features;
> + size_t mask;
> + int ret;
> +
> + if (!pci_epc_function_is_valid(epc, func_no, vfunc_no))
> + return -EINVAL;
> +
> + if (!size || !map)
> + return -EINVAL;
> +
> + memset(map, 0, sizeof(*map));
> + map->pci_addr = pci_addr;
> + map->pci_size = size;
> +
> + if (epc->ops->map_align) {
> + mutex_lock(&epc->lock);
> + ret = epc->ops->map_align(epc, func_no, vfunc_no, map);
> + mutex_unlock(&epc->lock);
> + return ret;
> + }
> +
> + /*
> + * Assume a fixed alignment constraint as specified by the controller
> + * features.
> + */
> + features = pci_epc_get_features(epc, func_no, vfunc_no);
> + if (!features || !features->align) {
> + map->map_pci_addr = pci_addr;
> + map->map_size = size;
> + map->map_ofst = 0;
> + }
The 'align' of pci_epc_features was initially added only to address the
inbound ATU constraints. This is also added as comment in [1]. The PCI
address restrictions (only fixed alignment constraint) were handled by
the host side driver and depends on the connected endpoint device
(atleast it was like that for pci_endpoint_test.c [2]).
So pci-epf-test.c used the 'align' in pci_epc_features only as part of
pci_epf_alloc_space().
Though I have abused 'align' of pci_epc_features in pci-epf-ntb.c using
it out of pci_epf_alloc_space(), I think we should keep the 'align' of
pci_epc_features only within pci_epf_alloc_space() and controllers with
any PCI address restrictions to implement ->map_align(). This could as
well be done in a phased manner to let controllers implement
->map_align() and then remove using pci_epc_features in
pci_epc_map_align(). Let me know what you think?
Thanks,
Kishon
[1] ->
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/include/linux/pci-epc.h?h=v6.9-rc2#n187
[2] ->
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/misc/pci_endpoint_test.c?h=v6.9-rc2#n127
> +
> + mask = features->align - 1;
> + map->map_pci_addr = map->pci_addr & ~mask;
> + map->map_ofst = map->pci_addr & mask;
> + map->map_size = ALIGN(map->map_ofst + map->pci_size, features->align);
> +
> + return 0;
> +}
> +EXPORT_SYMBOL_GPL(pci_epc_map_align);
> +
> /**
> * pci_epc_map_addr() - map CPU address to PCI address
> * @epc: the EPC device on which address is allocated
> diff --git a/include/linux/pci-epc.h b/include/linux/pci-epc.h
> index cc2f70d061c8..8cfb4aaf2628 100644
> --- a/include/linux/pci-epc.h
> +++ b/include/linux/pci-epc.h
> @@ -32,11 +32,40 @@ pci_epc_interface_string(enum pci_epc_interface_type type)
> }
> }
>
> +/**
> + * struct pci_epc_map - information about EPC memory for mapping a RC PCI
> + * address range
> + * @pci_addr: start address of the RC PCI address range to map
> + * @pci_size: size of the RC PCI address range to map
> + * @map_pci_addr: RC PCI address used as the first address mapped
> + * @map_size: size of the controller memory needed for the mapping
> + * @map_ofst: offset into the controller memory needed for the mapping
> + * @phys_base: base physical address of the allocated EPC memory
> + * @phys_addr: physical address at which @pci_addr is mapped
> + * @virt_base: base virtual address of the allocated EPC memory
> + * @virt_addr: virtual address at which @pci_addr is mapped
> + */
> +struct pci_epc_map {
> + phys_addr_t pci_addr;
> + size_t pci_size;
> +
> + phys_addr_t map_pci_addr;
> + size_t map_size;
> + phys_addr_t map_ofst;
> +
> + phys_addr_t phys_base;
> + phys_addr_t phys_addr;
> + void __iomem *virt_base;
> + void __iomem *virt_addr;
> +};
> +
> /**
> * struct pci_epc_ops - set of function pointers for performing EPC operations
> * @write_header: ops to populate configuration space header
> * @set_bar: ops to configure the BAR
> * @clear_bar: ops to reset the BAR
> + * @map_align: operation to get the size and offset into a controller memory
> + * window needed to map an RC PCI address region
> * @map_addr: ops to map CPU address to PCI address
> * @unmap_addr: ops to unmap CPU address and PCI address
> * @set_msi: ops to set the requested number of MSI interrupts in the MSI
> @@ -61,6 +90,8 @@ struct pci_epc_ops {
> struct pci_epf_bar *epf_bar);
> void (*clear_bar)(struct pci_epc *epc, u8 func_no, u8 vfunc_no,
> struct pci_epf_bar *epf_bar);
> + int (*map_align)(struct pci_epc *epc, u8 func_no, u8 vfunc_no,
> + struct pci_epc_map *map);
> int (*map_addr)(struct pci_epc *epc, u8 func_no, u8 vfunc_no,
> phys_addr_t addr, u64 pci_addr, size_t size);
> void (*unmap_addr)(struct pci_epc *epc, u8 func_no, u8 vfunc_no,
> @@ -234,6 +265,8 @@ int pci_epc_set_bar(struct pci_epc *epc, u8 func_no, u8 vfunc_no,
> struct pci_epf_bar *epf_bar);
> void pci_epc_clear_bar(struct pci_epc *epc, u8 func_no, u8 vfunc_no,
> struct pci_epf_bar *epf_bar);
> +int pci_epc_map_align(struct pci_epc *epc, u8 func_no, u8 vfunc_no,
> + u64 pci_addr, size_t size, struct pci_epc_map *map);
> int pci_epc_map_addr(struct pci_epc *epc, u8 func_no, u8 vfunc_no,
> phys_addr_t phys_addr,
> u64 pci_addr, size_t size);
^ permalink raw reply
* Re: [PATCH] media: mediatek: vcodec: fix the error sizeimage for 10bit bitstream
From: Sebastian Fricke @ 2024-04-03 12:39 UTC (permalink / raw)
To: Yunfei Dong
Cc: Nícolas F . R . A . Prado, Nicolas Dufresne, Hans Verkuil,
AngeloGioacchino Del Regno, Benjamin Gaignard, Nathan Hebert,
Hsin-Yi Wang, Fritz Koenig, Daniel Vetter, Steve Cho, linux-media,
devicetree, linux-kernel, linux-arm-kernel, linux-mediatek,
Project_Global_Chrome_Upstream_Group
In-Reply-To: <20240403093018.13168-1-yunfei.dong@mediatek.com>
Hey Yunfei,
On 03.04.2024 17:30, Yunfei Dong wrote:
>The sizeimage of each plane are calculated the same way for 8bit and
s/The sizeimage of each plane are/The sizeimage for each plane is/
>10bit bitstream. Need to enlarge the sizeimage with simeimage*5/4 for
>10bit bitstream when try and set fmt.
s/bitstream/bistreams/
s/Need to enlarge the sizeimage with simeimage*5/4 for 10bit bitstream when try and set fmt./
Scale up the sizeimage by 25% for 10-bit bitstreams in try_fmt./
>
>Fixes: 9d86be9bda6c ("media: mediatek: vcodec: Add driver to support 10bit")
>Signed-off-by: Yunfei Dong <yunfei.dong@mediatek.com>
>---
> .../mediatek/vcodec/decoder/mtk_vcodec_dec.c | 47 ++++++++++++++-----
> 1 file changed, 34 insertions(+), 13 deletions(-)
>
>diff --git a/drivers/media/platform/mediatek/vcodec/decoder/mtk_vcodec_dec.c b/drivers/media/platform/mediatek/vcodec/decoder/mtk_vcodec_dec.c
>index 9107707de6c4..45209894f1fe 100644
>--- a/drivers/media/platform/mediatek/vcodec/decoder/mtk_vcodec_dec.c
>+++ b/drivers/media/platform/mediatek/vcodec/decoder/mtk_vcodec_dec.c
>@@ -259,6 +259,7 @@ static int vidioc_try_fmt(struct mtk_vcodec_dec_ctx *ctx, struct v4l2_format *f,
> pix_fmt_mp->num_planes = 1;
> pix_fmt_mp->plane_fmt[0].bytesperline = 0;
> } else if (f->type == V4L2_BUF_TYPE_VIDEO_CAPTURE_MPLANE) {
>+ unsigned int dram_y, dram_c, dram_y_10bit, dram_c_10bit;
> int tmp_w, tmp_h;
>
> /*
>@@ -280,22 +281,42 @@ static int vidioc_try_fmt(struct mtk_vcodec_dec_ctx *ctx, struct v4l2_format *f,
> (pix_fmt_mp->height + 64) <= frmsize->max_height)
> pix_fmt_mp->height += 64;
>
>- mtk_v4l2_vdec_dbg(0, ctx,
>- "before resize wxh=%dx%d, after resize wxh=%dx%d, sizeimage=%d",
>- tmp_w, tmp_h, pix_fmt_mp->width, pix_fmt_mp->height,
>- pix_fmt_mp->width * pix_fmt_mp->height);
>+ dram_y = pix_fmt_mp->width * pix_fmt_mp->height;
>+ dram_c = dram_y / 2;
>+
>+ dram_y_10bit = dram_y * 5 / 4;
>+ dram_c_10bit = dram_y_10bit / 2;
I'd skip the two 10 bit variables (dram_y_10bit & dram_c_10bit) and
instead do it like this:
```
dram_stride = pix_fmt_mp->width;
if (ctx->is_10bit_bitstream)
dram_stride = dram_stride * 5 / 4;
dram_y = dram_stride * pix_fmt_mp->height;
dram_c = dram_y / 2;
if (pix_fmt_mp->num_planes == 1) {
pix_fmt_mp->plane_fmt[0].bytesperline = dram_stride;
pix_fmt_mp->plane_fmt[0].sizeimage = dram_y + dram_c;
} else {
pix_fmt_mp->plane_fmt[0].bytesperline = dram_stride;
pix_fmt_mp->plane_fmt[1].bytesperline = dram_stride;
pix_fmt_mp->plane_fmt[0].sizeimage = dram_y;
pix_fmt_mp->plane_fmt[1].sizeimage = dram_c;
...
}
```
Also, why do you call all the variables dram?
Please this isn't tested, but shows the general direction to repeat a
lot less code.
Greetings,
Sebastian
>
> pix_fmt_mp->num_planes = fmt->num_planes;
>- pix_fmt_mp->plane_fmt[0].sizeimage =
>- pix_fmt_mp->width * pix_fmt_mp->height;
>- pix_fmt_mp->plane_fmt[0].bytesperline = pix_fmt_mp->width;
>-
>- if (pix_fmt_mp->num_planes == 2) {
>- pix_fmt_mp->plane_fmt[1].sizeimage =
>- (pix_fmt_mp->width * pix_fmt_mp->height) / 2;
>- pix_fmt_mp->plane_fmt[1].bytesperline =
>- pix_fmt_mp->width;
>+ if (pix_fmt_mp->num_planes == 1) {
>+ if (ctx->is_10bit_bitstream) {
>+ pix_fmt_mp->plane_fmt[0].bytesperline = pix_fmt_mp->width * 5 / 4;
>+ pix_fmt_mp->plane_fmt[0].sizeimage = dram_y_10bit + dram_c_10bit;
>+ } else {
>+ pix_fmt_mp->plane_fmt[0].bytesperline = pix_fmt_mp->width;
>+ pix_fmt_mp->plane_fmt[0].sizeimage = dram_y + dram_c;
>+ }
>+ } else {
>+ if (ctx->is_10bit_bitstream) {
>+ pix_fmt_mp->plane_fmt[0].bytesperline = pix_fmt_mp->width * 5 / 4;
>+ pix_fmt_mp->plane_fmt[1].bytesperline = pix_fmt_mp->width * 5 / 4;
>+
>+ pix_fmt_mp->plane_fmt[0].sizeimage = dram_y_10bit;
>+ pix_fmt_mp->plane_fmt[1].sizeimage = dram_c_10bit;
>+ } else {
>+ pix_fmt_mp->plane_fmt[0].bytesperline = pix_fmt_mp->width;
>+ pix_fmt_mp->plane_fmt[1].bytesperline = pix_fmt_mp->width;
>+
>+ pix_fmt_mp->plane_fmt[0].sizeimage = dram_y;
>+ pix_fmt_mp->plane_fmt[1].sizeimage = dram_c;
>+ }
> }
>+
>+ mtk_v4l2_vdec_dbg(0, ctx,
>+ "before resize:%dx%d, after resize:%dx%d, sizeimage=0x%x_0x%x",
>+ tmp_w, tmp_h, pix_fmt_mp->width, pix_fmt_mp->height,
>+ pix_fmt_mp->plane_fmt[0].sizeimage,
>+ pix_fmt_mp->plane_fmt[1].sizeimage);
> }
>
> pix_fmt_mp->flags = 0;
>--
>2.25.1
>
^ permalink raw reply
* Re: [RFC PATCH 5/6] watchdog: ROHM BD96801 PMIC WDG driver
From: Guenter Roeck @ 2024-04-03 12:41 UTC (permalink / raw)
To: Matti Vaittinen
Cc: Matti Vaittinen, Lee Jones, Wim Van Sebroeck, devicetree,
linux-kernel, linux-watchdog
In-Reply-To: <279336b3-f28d-48ee-a10f-47abba7b0b89@gmail.com>
On Wed, Apr 03, 2024 at 09:34:35AM +0300, Matti Vaittinen wrote:
> Hi Guenter,
>
> First of all, thanks for the review. It was quick! Especially when we speak
> of a RFC series. Very much appreciated.
>
> On 4/2/24 20:11, Guenter Roeck wrote:
> > On Tue, Apr 02, 2024 at 04:11:41PM +0300, Matti Vaittinen wrote >> +static int init_wdg_hw(struct wdtbd96801 *w)
> > > +{
> > > + u32 hw_margin[2];
> > > + int count, ret;
> > > + u32 hw_margin_max = BD96801_WDT_DEFAULT_MARGIN, hw_margin_min = 0;
> > > +
> > > + count = device_property_count_u32(w->dev->parent, "rohm,hw-timeout-ms");
> > > + if (count < 0 && count != -EINVAL)
> > > + return count;
> > > +
> > > + if (count > 0) {
> > > + if (count > ARRAY_SIZE(hw_margin))
> > > + return -EINVAL;
> > > +
> > > + ret = device_property_read_u32_array(w->dev->parent,
> > > + "rohm,hw-timeout-ms",
> > > + &hw_margin[0], count);
> > > + if (ret < 0)
> > > + return ret;
> > > +
> > > + if (count == 1)
> > > + hw_margin_max = hw_margin[0];
> > > +
> > > + if (count == 2) {
> > > + hw_margin_max = hw_margin[1];
> > > + hw_margin_min = hw_margin[0];
> > > + }
> > > + }
> > > +
> > > + ret = bd96801_set_wdt_mode(w, hw_margin_max, hw_margin_min);
> > > + if (ret)
> > > + return ret;
> > > +
> > > + ret = device_property_match_string(w->dev->parent, "rohm,wdg-action",
> > > + "prstb");
> > > + if (ret >= 0) {
> > > + ret = regmap_update_bits(w->regmap, BD96801_REG_WD_CONF,
> > > + BD96801_WD_ASSERT_MASK,
> > > + BD96801_WD_ASSERT_RST);
> > > + return ret;
> > > + }
> > > +
> > > + ret = device_property_match_string(w->dev->parent, "rohm,wdg-action",
> > > + "intb-only");
> > > + if (ret >= 0) {
> > > + ret = regmap_update_bits(w->regmap, BD96801_REG_WD_CONF,
> > > + BD96801_WD_ASSERT_MASK,
> > > + BD96801_WD_ASSERT_IRQ);
> > > + return ret;
> > > + }
> >
> > I don't see the devicetree bindings documented in the series.
>
> Seems like I have missed this WDG binding. But after reading your comment
> below, I am wondering if I should just drop the binding and default to
> "prstb" (shutdown should the feeding be skipped) - and leave the "intb-only"
> case for one who actually needs such.
>
> > I am also a bit surprised that the interrupt isn't handled in the driver.
> > Please explain.
>
> Basically, I just had no idea what the IRQ should do in the generic case. If
> we get an interrupt, it means the WDG feeding has failed. My thinking is
> that, what should happen is forced reset. I don't see how that can be done
> in reliably manner from an IRQ handler.
>
> When the "prstb WDG action" is set (please, see the above DT binding
> handling), the PMIC shall shut down power outputs. This should get the
> watchdog's job done.
>
> With the "intb-only"-option, PMIC will not turn off the power. I'd expect
> there to be some external HW connection which handles the reset by HW.
>
> After all this being said, I wonder if I should just unconditionally
> configure the PMIC to always turn off the power (prstb option) should the
> feeding fail? Or do someone have some suggestion what the IRQ handler should
> do (except maybe print an error msg)?
>
Other watchdog drivers call emergency_restart() if the watchdog times out
and triggers an interrupt. Are you saying this won't work for this system ?
If so, please explain.
Thanks,
Guenter
^ permalink raw reply
* Re: [RFC PATCH 5/6] watchdog: ROHM BD96801 PMIC WDG driver
From: Matti Vaittinen @ 2024-04-03 12:47 UTC (permalink / raw)
To: Guenter Roeck
Cc: Matti Vaittinen, Lee Jones, Wim Van Sebroeck, devicetree,
linux-kernel, linux-watchdog
In-Reply-To: <d2ab33e6-4d3e-472a-b4d7-b703955989ba@roeck-us.net>
On 4/3/24 15:41, Guenter Roeck wrote:
> On Wed, Apr 03, 2024 at 09:34:35AM +0300, Matti Vaittinen wrote:
>> Hi Guenter,
>>
>> First of all, thanks for the review. It was quick! Especially when we speak
>> of a RFC series. Very much appreciated.
>>
>> On 4/2/24 20:11, Guenter Roeck wrote:
>>> On Tue, Apr 02, 2024 at 04:11:41PM +0300, Matti Vaittinen wrote >> +static int init_wdg_hw(struct wdtbd96801 *w)
>>>> +{
>>>> + u32 hw_margin[2];
>>>> + int count, ret;
>>>> + u32 hw_margin_max = BD96801_WDT_DEFAULT_MARGIN, hw_margin_min = 0;
>>>> +
>>>> + count = device_property_count_u32(w->dev->parent, "rohm,hw-timeout-ms");
>>>> + if (count < 0 && count != -EINVAL)
>>>> + return count;
>>>> +
>>>> + if (count > 0) {
>>>> + if (count > ARRAY_SIZE(hw_margin))
>>>> + return -EINVAL;
>>>> +
>>>> + ret = device_property_read_u32_array(w->dev->parent,
>>>> + "rohm,hw-timeout-ms",
>>>> + &hw_margin[0], count);
>>>> + if (ret < 0)
>>>> + return ret;
>>>> +
>>>> + if (count == 1)
>>>> + hw_margin_max = hw_margin[0];
>>>> +
>>>> + if (count == 2) {
>>>> + hw_margin_max = hw_margin[1];
>>>> + hw_margin_min = hw_margin[0];
>>>> + }
>>>> + }
>>>> +
>>>> + ret = bd96801_set_wdt_mode(w, hw_margin_max, hw_margin_min);
>>>> + if (ret)
>>>> + return ret;
>>>> +
>>>> + ret = device_property_match_string(w->dev->parent, "rohm,wdg-action",
>>>> + "prstb");
>>>> + if (ret >= 0) {
>>>> + ret = regmap_update_bits(w->regmap, BD96801_REG_WD_CONF,
>>>> + BD96801_WD_ASSERT_MASK,
>>>> + BD96801_WD_ASSERT_RST);
>>>> + return ret;
>>>> + }
>>>> +
>>>> + ret = device_property_match_string(w->dev->parent, "rohm,wdg-action",
>>>> + "intb-only");
>>>> + if (ret >= 0) {
>>>> + ret = regmap_update_bits(w->regmap, BD96801_REG_WD_CONF,
>>>> + BD96801_WD_ASSERT_MASK,
>>>> + BD96801_WD_ASSERT_IRQ);
>>>> + return ret;
>>>> + }
>>>
>>> I don't see the devicetree bindings documented in the series.
>>
>> Seems like I have missed this WDG binding. But after reading your comment
>> below, I am wondering if I should just drop the binding and default to
>> "prstb" (shutdown should the feeding be skipped) - and leave the "intb-only"
>> case for one who actually needs such.
>>
>>> I am also a bit surprised that the interrupt isn't handled in the driver.
>>> Please explain.
>>
>> Basically, I just had no idea what the IRQ should do in the generic case. If
>> we get an interrupt, it means the WDG feeding has failed. My thinking is
>> that, what should happen is forced reset. I don't see how that can be done
>> in reliably manner from an IRQ handler.
>>
>> When the "prstb WDG action" is set (please, see the above DT binding
>> handling), the PMIC shall shut down power outputs. This should get the
>> watchdog's job done.
>>
>> With the "intb-only"-option, PMIC will not turn off the power. I'd expect
>> there to be some external HW connection which handles the reset by HW.
>>
>> After all this being said, I wonder if I should just unconditionally
>> configure the PMIC to always turn off the power (prstb option) should the
>> feeding fail? Or do someone have some suggestion what the IRQ handler should
>> do (except maybe print an error msg)?
>>
>
> Other watchdog drivers call emergency_restart() if the watchdog times out
> and triggers an interrupt. Are you saying this won't work for this system ?
> If so, please explain.
>
Thanks Guenter. If it works with systems using other devices, then it
should work (to the same extent) with systems using this PMIC. Thanks.
I'll add the IRQ handling to next version - but it may take a while as
I'm currently having some problems with the IRQs in general, and because
I'll wait for feedback from Mark to the regulator part.
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 v2 2/2] ASoC: cdns: Add drivers of Cadence Multi-Channel I2S Controller
From: Pierre-Louis Bossart @ 2024-04-02 13:57 UTC (permalink / raw)
To: Xingyu Wu, Liam Girdwood, Mark Brown, Claudiu Beznea,
Jaroslav Kysela, Takashi Iwai, Rob Herring, Krzysztof Kozlowski,
Conor Dooley
Cc: devicetree@vger.kernel.org, linux-kernel@vger.kernel.org,
alsa-devel@alsa-project.org, linux-sound@vger.kernel.org
In-Reply-To: <NTZPR01MB0956BFADB4B3DA507D938F669F35A@NTZPR01MB0956.CHNPR01.prod.partner.outlook.cn>
>>
>>> +#define PERIODS_MIN 2
>>> +
>>> +static unsigned int cdns_i2s_pcm_tx(struct cdns_i2s_dev *dev,
>>> + struct snd_pcm_runtime *runtime,
>>> + unsigned int tx_ptr, bool *period_elapsed,
>>> + snd_pcm_format_t format)
>>> +{
>>> + unsigned int period_pos = tx_ptr % runtime->period_size;
>>
>> not following what the modulo is for, usually it's modulo the buffer size?
>
> This is to see if the new data is divisible by period_size and to determine whether
> it is enough for a period_size in the later loop.
That didn't answer to my question, the position is usually between
0..buffer_size.1.
Doing increments on a modulo value then comparisons as done below seems
rather questionable.
>>> +
>>> + iowrite32(data[0], dev->base + CDNS_FIFO_MEM);
>>> + iowrite32(data[1], dev->base + CDNS_FIFO_MEM);
>>> + period_pos++;
>>> + if (++tx_ptr >= runtime->buffer_size)
>>> + tx_ptr = 0;
>>> + }
>>> +
>>> + *period_elapsed = period_pos >= runtime->period_size;
>>> + return tx_ptr;
>>> +}
>>> + pm_runtime_enable(&pdev->dev);
>>> + if (pm_runtime_enabled(&pdev->dev))
>>> + cdns_i2s_runtime_suspend(&pdev->dev);
>>
>> that sequence looks suspicious.... Why would you suspend immediately during the
>> probe? You're probably missing all the autosuspend stuff?
>
> Since I have enabled clocks before, and the device is in the suspend state after
> pm_runtime_enable(), I need to disable clocks in cdns_i2s_runtime_suspend()
> to match the suspend state.
That is very odd on two counts
a) if you haven't enabled the clocks, why do you need to disbale them?
b) if you do a pm_runtime_enable(), then the branch if
(pm_runtime_enabled) is always true.
>
>>
>>> +
>>> + dev_dbg(&pdev->dev, "I2S supports %d stereo channels with %s.\n",
>>> + i2s->max_channels, ((i2s->irq < 0) ? "dma" : "interrupt"));
>>> +
>>> + return 0;
>>> +
>>> +err:
>>> + return ret;
>>> +}
>>> +
>>> +static int cdns_i2s_remove(struct platform_device *pdev) {
>>> + pm_runtime_disable(&pdev->dev);
>>> + if (!pm_runtime_status_suspended(&pdev->dev))
>>> + cdns_i2s_runtime_suspend(&pdev->dev);
>>
>> ... and this one too. Once you've disabled pm_runtime, checking the status is
>> irrelevant...
>
> I think the clocks need to be always enabled after probe if disable pm_runtime,
> and should be disabled when remove. This will do that.
if you are disabling pm_runtime, then the pm_runtime state becames invalid.
When pm_runtime_disable() is added in remove operations, it's mainly to
prevent the device from suspending.
^ permalink raw reply
* Re: [PATCH RFT 01/10] arm64: dts: microchip: sparx5: fix mdio reg
From: Steen Hegelund @ 2024-04-03 13:03 UTC (permalink / raw)
To: Conor Dooley
Cc: Krzysztof Kozlowski, Nicolas Ferre, Claudiu Beznea, Rob Herring,
Krzysztof Kozlowski, Lars Povlsen, Daniel Machon, UNGLinuxDriver,
David S. Miller, Bjarni Jonasson, linux-arm-kernel, devicetree,
linux-kernel
In-Reply-To: <20240402-drizzly-risotto-eac556bbe95b@spud>
Hi Connor,
On Tue, 2024-04-02 at 18:46 +0100, Conor Dooley wrote:
> [Some people who received this message don't often get email from
> conor@kernel.org. Learn why this is important at
> https://aka.ms/LearnAboutSenderIdentification ]
>
> EXTERNAL EMAIL: Do not click links or open attachments unless you
> know the content is safe
I will also be looking at the other RFT marked patches, I just have not
had the time to do it yet...
BR
Steen
^ permalink raw reply
* Re: [PATCH v2 3/3] arm64: dts: qcom: msm8998: set qcom,no-msa-ready-indicator for wifi
From: Marc Gonzalez @ 2024-04-03 13:05 UTC (permalink / raw)
To: Dmitry Baryshkov
Cc: Konrad Dybcio, Krzysztof Kozlowski, Kalle Valo, Jeff Johnson,
ath10k, wireless, DT, MSM, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, Pierre-Hugues Husson, Arnaud Vrac, Bjorn Andersson,
Jami Kettunen, Jeffrey Hugo
In-Reply-To: <CAA8EJppeREj-0g9oGCzzKx5ywhg1mgmJR1q8yvXKN7N45do1Xg@mail.gmail.com>
On 02/04/2024 17:55, Dmitry Baryshkov wrote:
> On Tue, 2 Apr 2024 at 18:31, Marc Gonzalez wrote:
>
>> So, if I understand correctly, I take this to mean that I should:
>>
>> 1) DELETE the qcom,no-msa-ready-indicator boolean property,
>> 2) ADD a "qcom,msm8998-wifi" (name OK?) compatible,
>
> I'd say, this is not correct. There is no "msm8998-wifi".
Can you explain what you mean by:
'There is no "msm8998-wifi".' ?
Do you mean that: this compatible string does not exist?
(I am proposing that it be created.)
Or do you mean that: "msm8998-wifi" is a bad name?
I meant to mimic these strings for various sub-blocks:
arch/arm64/boot/dts/qcom/msm8998.dtsi: compatible = "qcom,msm8998-rpm-proc", "qcom,rpm-proc";
arch/arm64/boot/dts/qcom/msm8998.dtsi: compatible = "qcom,msm8998-rpmpd";
arch/arm64/boot/dts/qcom/msm8998.dtsi: compatible = "qcom,msm8998-qfprom", "qcom,qfprom";
arch/arm64/boot/dts/qcom/msm8998.dtsi: compatible = "qcom,msm8998-tsens", "qcom,tsens-v2";
arch/arm64/boot/dts/qcom/msm8998.dtsi: compatible = "qcom,msm8998-tsens", "qcom,tsens-v2";
arch/arm64/boot/dts/qcom/msm8998.dtsi: compatible = "qcom,msm8998-smmu-v2", "qcom,smmu-v2";
arch/arm64/boot/dts/qcom/msm8998.dtsi: compatible = "qcom,msm8998-smmu-v2", "qcom,smmu-v2";
arch/arm64/boot/dts/qcom/msm8998.dtsi: compatible = "qcom,msm8998-qmp-pcie-phy";
arch/arm64/boot/dts/qcom/msm8998.dtsi: compatible = "qcom,msm8998-ufshc", "qcom,ufshc", "jedec,ufs-2.0";
arch/arm64/boot/dts/qcom/msm8998.dtsi: compatible = "qcom,msm8998-qmp-ufs-phy";
arch/arm64/boot/dts/qcom/msm8998.dtsi: compatible = "qcom,msm8998-tcsr", "syscon";
arch/arm64/boot/dts/qcom/msm8998.dtsi: compatible = "qcom,msm8998-tcsr", "syscon";
arch/arm64/boot/dts/qcom/msm8998.dtsi: compatible = "qcom,msm8998-pinctrl";
arch/arm64/boot/dts/qcom/msm8998.dtsi: compatible = "qcom,msm8998-mss-pil";
arch/arm64/boot/dts/qcom/msm8998.dtsi: compatible = "qcom,msm8998-smmu-v2", "qcom,smmu-v2",
arch/arm64/boot/dts/qcom/msm8998.dtsi: compatible = "qcom,msm8998-gpucc";
arch/arm64/boot/dts/qcom/msm8998.dtsi: compatible = "qcom,msm8998-slpi-pas";
arch/arm64/boot/dts/qcom/msm8998.dtsi: compatible = "qcom,msm8998-dwc3", "qcom,dwc3";
arch/arm64/boot/dts/qcom/msm8998.dtsi: compatible = "qcom,msm8998-qmp-usb3-phy";
arch/arm64/boot/dts/qcom/msm8998.dtsi: compatible = "qcom,msm8998-qusb2-phy";
arch/arm64/boot/dts/qcom/msm8998.dtsi: compatible = "qcom,msm8998-sdhci", "qcom,sdhci-msm-v4";
arch/arm64/boot/dts/qcom/msm8998.dtsi: compatible = "qcom,msm8998-mdss";
arch/arm64/boot/dts/qcom/msm8998.dtsi: compatible = "qcom,msm8998-dpu";
arch/arm64/boot/dts/qcom/msm8998.dtsi: compatible = "qcom,msm8998-dsi-ctrl", "qcom,mdss-dsi-ctrl";
arch/arm64/boot/dts/qcom/msm8998.dtsi: compatible = "qcom,msm8998-dsi-ctrl", "qcom,mdss-dsi-ctrl";
arch/arm64/boot/dts/qcom/msm8998.dtsi: compatible = "qcom,msm8998-venus";
arch/arm64/boot/dts/qcom/msm8998.dtsi: compatible = "qcom,msm8998-smmu-v2", "qcom,smmu-v2";
arch/arm64/boot/dts/qcom/msm8998.dtsi: compatible = "qcom,msm8998-adsp-pas";
arch/arm64/boot/dts/qcom/msm8998.dtsi: compatible = "qcom,msm8998-apcs-hmss-global",
And these strings in ath11k:
Documentation/devicetree/bindings/net/wireless/qcom,ath11k.yaml: - qcom,ipq8074-wifi
Documentation/devicetree/bindings/net/wireless/qcom,ath11k.yaml: - qcom,ipq6018-wifi
Documentation/devicetree/bindings/net/wireless/qcom,ath11k.yaml: - qcom,wcn6750-wifi
Documentation/devicetree/bindings/net/wireless/qcom,ath11k.yaml: - qcom,ipq5018-wifi
> I'd say, we should take a step back and actually verify how this was
> handled in the vendor kernel.
In our commercial product, we use the ath10k driver in the vendor kernel (v4.4 r38-rel).
It looks like Jeff has already performed the code analysis
wrt vendor vs mainline (including user-space tools).
Regards
^ permalink raw reply
* Re: [RFC PATCH 5/6] watchdog: ROHM BD96801 PMIC WDG driver
From: Guenter Roeck @ 2024-04-03 13:26 UTC (permalink / raw)
To: Matti Vaittinen
Cc: Matti Vaittinen, Lee Jones, Wim Van Sebroeck, devicetree,
linux-kernel, linux-watchdog
In-Reply-To: <1d956aab-2892-4a2b-a4b3-0a93504668eb@gmail.com>
On Wed, Apr 03, 2024 at 03:47:12PM +0300, Matti Vaittinen wrote:
> >
> > Other watchdog drivers call emergency_restart() if the watchdog times out
> > and triggers an interrupt. Are you saying this won't work for this system ?
> > If so, please explain.
> >
>
> Thanks Guenter. If it works with systems using other devices, then it should
> work (to the same extent) with systems using this PMIC. Thanks.
>
You might also consider to just call panic(). What is what we do if the
pretimeout panic governor is enabled.
That makes me wonder if it would make sense to introduce watchdog timeout
governors, similar to the existing pretimeout governors. Maybe if I ever
find the time to do it ...
Guenter
> I'll add the IRQ handling to next version - but it may take a while as I'm
> currently having some problems with the IRQs in general, and because I'll
> wait for feedback from Mark to the regulator part.
>
> 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
* [PATCH RESEND] arm64: dts: qcom: qcs6490-rb3gen2: enable PMIC Volume and Power buttons
From: Umang Chheda @ 2024-04-03 13:28 UTC (permalink / raw)
To: Bjorn Andersson, Konrad Dybcio, Rob Herring, Krzysztof Kozlowski,
Conor Dooley
Cc: linux-arm-msm, devicetree, linux-kernel, quic_uchheda
The Volume Down & Power buttons are controlled by the PMIC via
the PON hardware, and the Volume Up is connected to a PMIC gpio.
Enable the necessary hardware and setup the GPIO state for the
Volume Up gpio key.
Signed-off-by: Umang Chheda <quic_uchheda@quicinc.com>
---
arch/arm64/boot/dts/qcom/qcs6490-rb3gen2.dts | 37 ++++++++++++++++++++
1 file changed, 37 insertions(+)
diff --git a/arch/arm64/boot/dts/qcom/qcs6490-rb3gen2.dts b/arch/arm64/boot/dts/qcom/qcs6490-rb3gen2.dts
index 63ebe0774f1d..73f6d18d2331 100644
--- a/arch/arm64/boot/dts/qcom/qcs6490-rb3gen2.dts
+++ b/arch/arm64/boot/dts/qcom/qcs6490-rb3gen2.dts
@@ -9,6 +9,8 @@
#define PM7250B_SID 8
#define PM7250B_SID1 9
+#include <dt-bindings/input/linux-event-codes.h>
+#include <dt-bindings/pinctrl/qcom,pmic-gpio.h>
#include <dt-bindings/regulator/qcom,rpmh-regulator.h>
#include "sc7280.dtsi"
#include "pm7250b.dtsi"
@@ -39,6 +41,22 @@ chosen {
stdout-path = "serial0:115200n8";
};
+ gpio-keys {
+ compatible = "gpio-keys";
+
+ pinctrl-0 = <&key_vol_up_default>;
+ pinctrl-names = "default";
+
+ key-volume-up {
+ label = "Volume_up";
+ gpios = <&pm7325_gpios 6 GPIO_ACTIVE_LOW>;
+ linux,code = <KEY_VOLUMEUP>;
+ wakeup-source;
+ debounce-interval = <15>;
+ linux,can-disable;
+ };
+ };
+
reserved-memory {
xbl_mem: xbl@80700000 {
reg = <0x0 0x80700000 0x0 0x100000>;
@@ -471,6 +489,25 @@ &gcc {
<GCC_WPSS_RSCP_CLK>;
};
+&pm7325_gpios {
+ key_vol_up_default: key-vol-up-state {
+ pins = "gpio6";
+ function = "normal";
+ input-enable;
+ bias-pull-up;
+ qcom,drive-strength = <PMIC_GPIO_STRENGTH_LOW>;
+ };
+};
+
+&pon_pwrkey {
+ status = "okay";
+};
+
+&pon_resin {
+ linux,code = <KEY_VOLUMEDOWN>;
+ status = "okay";
+};
+
&qupv3_id_0 {
status = "okay";
};
--
2.25.1
^ permalink raw reply related
* [PATCH RESEND] arm64: dts: qcom: qcm6490-idp: Name the regulators
From: Umang Chheda @ 2024-04-03 13:29 UTC (permalink / raw)
To: Bjorn Andersson, Konrad Dybcio, Rob Herring, Krzysztof Kozlowski,
Conor Dooley
Cc: linux-arm-msm, devicetree, linux-kernel, quic_uchheda
Without explicitly specifying names for the regulators they are named
based on the DeviceTree node name. This results in multiple regulators
with the same name, making it impossible to reason debug prints and
regulator_summary.
Signed-off-by: Umang Chheda <quic_uchheda@quicinc.com>
---
arch/arm64/boot/dts/qcom/qcm6490-idp.dts | 41 ++++++++++++++++++++++++
1 file changed, 41 insertions(+)
diff --git a/arch/arm64/boot/dts/qcom/qcm6490-idp.dts b/arch/arm64/boot/dts/qcom/qcm6490-idp.dts
index f8f8a43f638d..ac6d741868ca 100644
--- a/arch/arm64/boot/dts/qcom/qcm6490-idp.dts
+++ b/arch/arm64/boot/dts/qcom/qcm6490-idp.dts
@@ -195,129 +195,151 @@ regulators-0 {
vdd-l14-l16-supply = <&vreg_s8b_1p272>;
vreg_s1b_1p872: smps1 {
+ regulator-name = "vreg_s1b_1p872";
regulator-min-microvolt = <1840000>;
regulator-max-microvolt = <2040000>;
};
vreg_s2b_0p876: smps2 {
+ regulator-name = "vreg_s2b_0p876";
regulator-min-microvolt = <570070>;
regulator-max-microvolt = <1050000>;
};
vreg_s7b_0p972: smps7 {
+ regulator-name = "vreg_s7b_0p972";
regulator-min-microvolt = <535000>;
regulator-max-microvolt = <1120000>;
};
vreg_s8b_1p272: smps8 {
+ regulator-name = "vreg_s8b_1p272";
regulator-min-microvolt = <1200000>;
regulator-max-microvolt = <1500000>;
regulator-initial-mode = <RPMH_REGULATOR_MODE_RET>;
};
vreg_l1b_0p912: ldo1 {
+ regulator-name = "vreg_l1b_0p912";
regulator-min-microvolt = <825000>;
regulator-max-microvolt = <925000>;
regulator-initial-mode = <RPMH_REGULATOR_MODE_HPM>;
};
vreg_l2b_3p072: ldo2 {
+ regulator-name = "vreg_l2b_3p072";
regulator-min-microvolt = <2700000>;
regulator-max-microvolt = <3544000>;
regulator-initial-mode = <RPMH_REGULATOR_MODE_HPM>;
};
vreg_l3b_0p504: ldo3 {
+ regulator-name = "vreg_l3b_0p504";
regulator-min-microvolt = <312000>;
regulator-max-microvolt = <910000>;
regulator-initial-mode = <RPMH_REGULATOR_MODE_HPM>;
};
vreg_l4b_0p752: ldo4 {
+ regulator-name = "vreg_l4b_0p752";
regulator-min-microvolt = <752000>;
regulator-max-microvolt = <820000>;
regulator-initial-mode = <RPMH_REGULATOR_MODE_HPM>;
};
reg_l5b_0p752: ldo5 {
+ regulator-name = "reg_l5b_0p752";
regulator-min-microvolt = <552000>;
regulator-max-microvolt = <832000>;
regulator-initial-mode = <RPMH_REGULATOR_MODE_HPM>;
};
vreg_l6b_1p2: ldo6 {
+ regulator-name = "vreg_l6b_1p2";
regulator-min-microvolt = <1140000>;
regulator-max-microvolt = <1260000>;
regulator-initial-mode = <RPMH_REGULATOR_MODE_HPM>;
};
vreg_l7b_2p952: ldo7 {
+ regulator-name = "vreg_l7b_2p952";
regulator-min-microvolt = <2400000>;
regulator-max-microvolt = <3544000>;
regulator-initial-mode = <RPMH_REGULATOR_MODE_HPM>;
};
vreg_l8b_0p904: ldo8 {
+ regulator-name = "vreg_l8b_0p904";
regulator-min-microvolt = <870000>;
regulator-max-microvolt = <970000>;
regulator-initial-mode = <RPMH_REGULATOR_MODE_HPM>;
};
vreg_l9b_1p2: ldo9 {
+ regulator-name = "vreg_l9b_1p2";
regulator-min-microvolt = <1200000>;
regulator-max-microvolt = <1304000>;
regulator-initial-mode = <RPMH_REGULATOR_MODE_HPM>;
};
vreg_l11b_1p504: ldo11 {
+ regulator-name = "vreg_l11b_1p504";
regulator-min-microvolt = <1504000>;
regulator-max-microvolt = <2000000>;
regulator-initial-mode = <RPMH_REGULATOR_MODE_HPM>;
};
vreg_l12b_0p751: ldo12 {
+ regulator-name = "vreg_l12b_0p751";
regulator-min-microvolt = <751000>;
regulator-max-microvolt = <824000>;
regulator-initial-mode = <RPMH_REGULATOR_MODE_HPM>;
};
vreg_l13b_0p53: ldo13 {
+ regulator-name = "vreg_l13b_0p53";
regulator-min-microvolt = <530000>;
regulator-max-microvolt = <824000>;
regulator-initial-mode = <RPMH_REGULATOR_MODE_HPM>;
};
vreg_l14b_1p08: ldo14 {
+ regulator-name = "vreg_l14b_1p08";
regulator-min-microvolt = <1080000>;
regulator-max-microvolt = <1304000>;
regulator-initial-mode = <RPMH_REGULATOR_MODE_HPM>;
};
vreg_l15b_0p765: ldo15 {
+ regulator-name = "vreg_l15b_0p765";
regulator-min-microvolt = <765000>;
regulator-max-microvolt = <1020000>;
regulator-initial-mode = <RPMH_REGULATOR_MODE_HPM>;
};
vreg_l16b_1p1: ldo16 {
+ regulator-name = "vreg_l16b_1p1";
regulator-min-microvolt = <1100000>;
regulator-max-microvolt = <1300000>;
regulator-initial-mode = <RPMH_REGULATOR_MODE_HPM>;
};
vreg_l17b_1p7: ldo17 {
+ regulator-name = "vreg_l17b_1p7";
regulator-min-microvolt = <1700000>;
regulator-max-microvolt = <1900000>;
regulator-initial-mode = <RPMH_REGULATOR_MODE_HPM>;
};
vreg_l18b_1p8: ldo18 {
+ regulator-name = "vreg_l18b_1p8";
regulator-min-microvolt = <1800000>;
regulator-max-microvolt = <2000000>;
regulator-initial-mode = <RPMH_REGULATOR_MODE_HPM>;
};
vreg_l19b_1p8: ldo19 {
+ regulator-name = "vreg_l19b_1p8";
regulator-min-microvolt = <1800000>;
regulator-max-microvolt = <2000000>;
regulator-initial-mode = <RPMH_REGULATOR_MODE_HPM>;
@@ -349,109 +371,128 @@ regulators-1 {
vdd-bob-supply = <&vph_pwr>;
vreg_s1c_2p19: smps1 {
+ regulator-name = "vreg_s1c_2p19";
regulator-min-microvolt = <2190000>;
regulator-max-microvolt = <2210000>;
};
vreg_s2c_0p752: smps2 {
+ regulator-name = "vreg_s2c_0p752";
regulator-min-microvolt = <750000>;
regulator-max-microvolt = <800000>;
};
vreg_s5c_0p752: smps5 {
+ regulator-name = "vreg_s5c_0p752";
regulator-min-microvolt = <465000>;
regulator-max-microvolt = <1050000>;
};
vreg_s7c_0p752: smps7 {
+ regulator-name = "vreg_s7c_0p752";
regulator-min-microvolt = <465000>;
regulator-max-microvolt = <800000>;
};
vreg_s9c_1p084: smps9 {
+ regulator-name = "vreg_s9c_1p084";
regulator-min-microvolt = <1010000>;
regulator-max-microvolt = <1170000>;
};
vreg_l1c_1p8: ldo1 {
+ regulator-name = "vreg_l1c_1p8";
regulator-min-microvolt = <1800000>;
regulator-max-microvolt = <1980000>;
regulator-initial-mode = <RPMH_REGULATOR_MODE_HPM>;
};
vreg_l2c_1p62: ldo2 {
+ regulator-name = "vreg_l2c_1p62";
regulator-min-microvolt = <1620000>;
regulator-max-microvolt = <1980000>;
regulator-initial-mode = <RPMH_REGULATOR_MODE_HPM>;
};
vreg_l3c_2p8: ldo3 {
+ regulator-name = "vreg_l3c_2p8";
regulator-min-microvolt = <2800000>;
regulator-max-microvolt = <3540000>;
regulator-initial-mode = <RPMH_REGULATOR_MODE_HPM>;
};
vreg_l4c_1p62: ldo4 {
+ regulator-name = "vreg_l4c_1p62";
regulator-min-microvolt = <1620000>;
regulator-max-microvolt = <3300000>;
regulator-initial-mode = <RPMH_REGULATOR_MODE_HPM>;
};
vreg_l5c_1p62: ldo5 {
+ regulator-name = "vreg_l5c_1p62";
regulator-min-microvolt = <1620000>;
regulator-max-microvolt = <3300000>;
regulator-initial-mode = <RPMH_REGULATOR_MODE_HPM>;
};
vreg_l6c_2p96: ldo6 {
+ regulator-name = "vreg_l6c_2p96";
regulator-min-microvolt = <1650000>;
regulator-max-microvolt = <3544000>;
regulator-initial-mode = <RPMH_REGULATOR_MODE_HPM>;
};
vreg_l7c_3p0: ldo7 {
+ regulator-name = "vreg_l7c_3p0";
regulator-min-microvolt = <3000000>;
regulator-max-microvolt = <3544000>;
regulator-initial-mode = <RPMH_REGULATOR_MODE_HPM>;
};
vreg_l8c_1p62: ldo8 {
+ regulator-name = "vreg_l8c_1p62";
regulator-min-microvolt = <1620000>;
regulator-max-microvolt = <2000000>;
regulator-initial-mode = <RPMH_REGULATOR_MODE_HPM>;
};
vreg_l9c_2p96: ldo9 {
+ regulator-name = "vreg_l9c_2p96";
regulator-min-microvolt = <2700000>;
regulator-max-microvolt = <35440000>;
regulator-initial-mode = <RPMH_REGULATOR_MODE_HPM>;
};
vreg_l10c_0p88: ldo10 {
+ regulator-name = "vreg_l10c_0p88";
regulator-min-microvolt = <720000>;
regulator-max-microvolt = <1050000>;
regulator-initial-mode = <RPMH_REGULATOR_MODE_HPM>;
};
vreg_l11c_2p8: ldo11 {
+ regulator-name = "vreg_l11c_2p8";
regulator-min-microvolt = <2800000>;
regulator-max-microvolt = <3544000>;
regulator-initial-mode = <RPMH_REGULATOR_MODE_HPM>;
};
vreg_l12c_1p65: ldo12 {
+ regulator-name = "vreg_l12c_1p65";
regulator-min-microvolt = <1650000>;
regulator-max-microvolt = <2000000>;
regulator-initial-mode = <RPMH_REGULATOR_MODE_HPM>;
};
vreg_l13c_2p7: ldo13 {
+ regulator-name = "vreg_l13c_2p7";
regulator-min-microvolt = <2700000>;
regulator-max-microvolt = <3544000>;
regulator-initial-mode = <RPMH_REGULATOR_MODE_HPM>;
};
vreg_bob_3p296: bob {
+ regulator-name = "vreg_bob_3p296";
regulator-min-microvolt = <3008000>;
regulator-max-microvolt = <3960000>;
};
--
2.25.1
^ permalink raw reply related
* [PATCH v2 3/3] ARM: dts: Modify I2C bus configuration
From: Renze Nicolai @ 2024-04-03 13:28 UTC (permalink / raw)
To: renze, linux-arm-kernel, devicetree, linux-kernel, linux-aspeed,
arnd, olof, soc, robh+dt, krzysztof.kozlowski+dt, joel, andrew
In-Reply-To: <20240403133037.37782-1-renze@rnplus.nl>
Enable I2C bus 8 which is exposed on the IPMB_1 connector
on the X570D4U mainboard.
Additionally adds a descriptive comment to I2C busses 1 and 5.
Signed-off-by: Renze Nicolai <renze@rnplus.nl>
---
arch/arm/boot/dts/aspeed/aspeed-bmc-asrock-x570d4u.dts | 7 +++++++
1 file changed, 7 insertions(+)
diff --git a/arch/arm/boot/dts/aspeed/aspeed-bmc-asrock-x570d4u.dts b/arch/arm/boot/dts/aspeed/aspeed-bmc-asrock-x570d4u.dts
index 66318ef8caae..8dee4faa9e07 100644
--- a/arch/arm/boot/dts/aspeed/aspeed-bmc-asrock-x570d4u.dts
+++ b/arch/arm/boot/dts/aspeed/aspeed-bmc-asrock-x570d4u.dts
@@ -162,6 +162,7 @@ &i2c0 {
};
&i2c1 {
+ /* Hardware monitoring SMBus */
status = "okay";
w83773g@4c {
@@ -219,6 +220,7 @@ i2c4mux0ch3: i2c@3 {
};
&i2c5 {
+ /* SMBus on BMC connector (BMC_SMB_1) */
status = "okay";
};
@@ -243,6 +245,11 @@ eth1_macaddress: macaddress@3f88 {
};
};
+&i2c8 {
+ /* SMBus on intelligent platform management bus header (IPMB_1) */
+ status = "okay";
+};
+
&gfx {
status = "okay";
};
--
2.44.0
^ permalink raw reply related
* [PATCH v2 1/3] ARM: dts: Modify GPIO table for Asrock X570D4U BMC
From: Renze Nicolai @ 2024-04-03 13:28 UTC (permalink / raw)
To: renze, linux-arm-kernel, devicetree, linux-kernel, linux-aspeed,
arnd, olof, soc, robh+dt, krzysztof.kozlowski+dt, joel, andrew
In-Reply-To: <20240403133037.37782-1-renze@rnplus.nl>
Restructure GPIO table to fit maximum line length.
Fix mistakes found while working on OpenBMC
userland configuration and based on probing
the board.
Schematic for this board is not available.
Because of this the choice was made to
use a descriptive method for naming the
GPIOs.
- Push-pull outputs start with output-*
- Open-drain outputs start with control-*
- LED outputs start with led-*
- Inputs start with input-*
- Button inputs start with button-*
- Active low signals end with *-n
Signed-off-by: Renze Nicolai <renze@rnplus.nl>
---
.../dts/aspeed/aspeed-bmc-asrock-x570d4u.dts | 95 ++++++++-----------
1 file changed, 37 insertions(+), 58 deletions(-)
diff --git a/arch/arm/boot/dts/aspeed/aspeed-bmc-asrock-x570d4u.dts b/arch/arm/boot/dts/aspeed/aspeed-bmc-asrock-x570d4u.dts
index 3c975bc41ae7..dff69d6ff146 100644
--- a/arch/arm/boot/dts/aspeed/aspeed-bmc-asrock-x570d4u.dts
+++ b/arch/arm/boot/dts/aspeed/aspeed-bmc-asrock-x570d4u.dts
@@ -79,64 +79,43 @@ iio-hwmon {
&gpio {
status = "okay";
gpio-line-names =
- /*A0-A3*/ "status-locatorled-n", "", "button-nmi-n", "",
- /*A4-A7*/ "", "", "", "",
- /*B0-B3*/ "input-bios-post-cmplt-n", "", "", "",
- /*B4-B7*/ "", "", "", "",
- /*C0-C3*/ "", "", "", "",
- /*C4-C7*/ "", "", "control-locatorbutton", "",
- /*D0-D3*/ "button-power", "control-power", "button-reset", "control-reset",
- /*D4-D7*/ "", "", "", "",
- /*E0-E3*/ "", "", "", "",
- /*E4-E7*/ "", "", "", "",
- /*F0-F3*/ "", "", "", "",
- /*F4-F7*/ "", "", "", "",
- /*G0-G3*/ "output-rtc-battery-voltage-read-enable", "input-id0", "input-id1", "input-id2",
- /*G4-G7*/ "input-alert1-n", "input-alert2-n", "input-alert3-n", "",
- /*H0-H3*/ "", "", "", "",
- /*H4-H7*/ "input-mfg", "", "led-heartbeat-n", "input-caseopen",
- /*I0-I3*/ "", "", "", "",
- /*I4-I7*/ "", "", "", "",
- /*J0-J3*/ "output-bmc-ready", "", "", "",
- /*J4-J7*/ "", "", "", "",
- /*K0-K3*/ "", "", "", "",
- /*K4-K7*/ "", "", "", "",
- /*L0-L3*/ "", "", "", "",
- /*L4-L7*/ "", "", "", "",
- /*M0-M3*/ "", "", "", "",
- /*M4-M7*/ "", "", "", "",
- /*N0-N3*/ "", "", "", "",
- /*N4-N7*/ "", "", "", "",
- /*O0-O3*/ "", "", "", "",
- /*O4-O7*/ "", "", "", "",
- /*P0-P3*/ "", "", "", "",
- /*P4-P7*/ "", "", "", "",
- /*Q0-Q3*/ "", "", "", "",
- /*Q4-Q7*/ "", "", "", "",
- /*R0-R3*/ "", "", "", "",
- /*R4-R7*/ "", "", "", "",
- /*S0-S3*/ "input-bmc-pchhot-n", "", "", "",
- /*S4-S7*/ "", "", "", "",
- /*T0-T3*/ "", "", "", "",
- /*T4-T7*/ "", "", "", "",
- /*U0-U3*/ "", "", "", "",
- /*U4-U7*/ "", "", "", "",
- /*V0-V3*/ "", "", "", "",
- /*V4-V7*/ "", "", "", "",
- /*W0-W3*/ "", "", "", "",
- /*W4-W7*/ "", "", "", "",
- /*X0-X3*/ "", "", "", "",
- /*X4-X7*/ "", "", "", "",
- /*Y0-Y3*/ "", "", "", "",
- /*Y4-Y7*/ "", "", "", "",
- /*Z0-Z3*/ "", "", "led-fault-n", "output-bmc-throttle-n",
- /*Z4-Z7*/ "", "", "", "",
- /*AA0-AA3*/ "input-cpu1-thermtrip-latch-n", "", "input-cpu1-prochot-n", "",
- /*AA4-AC7*/ "", "", "", "",
- /*AB0-AB3*/ "", "", "", "",
- /*AB4-AC7*/ "", "", "", "",
- /*AC0-AC3*/ "", "", "", "",
- /*AC4-AC7*/ "", "", "", "";
+ /* A */ "input-locatorled-n", "", "", "", "", "", "", "",
+ /* B */ "input-bios-post-cmplt-n", "", "", "", "", "", "", "",
+ /* C */ "", "", "", "", "", "", "control-locatorbutton-n", "",
+ /* D */ "button-power-n", "control-power-n", "button-reset-n",
+ "control-reset-n", "", "", "", "",
+ /* E */ "", "", "", "", "", "", "", "",
+ /* F */ "", "", "", "", "", "", "", "",
+ /* G */ "output-hwm-vbat-enable", "input-id0-n", "input-id1-n",
+ "input-id2-n", "input-aux-smb-alert-n", "",
+ "input-psu-smb-alert-n", "",
+ /* H */ "", "", "", "", "input-mfg-mode-n", "",
+ "led-heartbeat-n", "input-case-open-n",
+ /* I */ "", "", "", "", "", "", "", "",
+ /* J */ "output-bmc-ready-n", "", "", "", "", "", "", "",
+ /* K */ "", "", "", "", "", "", "", "",
+ /* L */ "", "", "", "", "", "", "", "",
+ /* M */ "", "", "", "", "", "", "", "",
+ /* N */ "", "", "", "", "", "", "", "",
+ /* O */ "", "", "", "", "", "", "", "",
+ /* P */ "", "", "", "", "", "", "", "",
+ /* Q */ "", "", "", "", "input-bmc-smb-present-n", "", "",
+ "input-pcie-wake-n",
+ /* R */ "", "", "", "", "", "", "", "",
+ /* S */ "input-bmc-pchhot-n", "", "", "", "", "", "", "",
+ /* T */ "", "", "", "", "", "", "", "",
+ /* U */ "", "", "", "", "", "", "", "",
+ /* V */ "", "", "", "", "", "", "", "",
+ /* W */ "", "", "", "", "", "", "", "",
+ /* X */ "", "", "", "", "", "", "", "",
+ /* Y */ "input-sleep-s3-n", "input-sleep-s5-n", "", "", "", "",
+ "", "",
+ /* Z */ "", "", "led-fault-n", "output-bmc-throttle-n", "", "",
+ "", "",
+ /* AA */ "input-cpu1-thermtrip-latch-n", "",
+ "input-cpu1-prochot-n", "", "", "", "", "",
+ /* AB */ "", "input-power-good", "", "", "", "", "", "",
+ /* AC */ "", "", "", "", "", "", "", "";
};
&fmc {
--
2.44.0
^ permalink raw reply related
* [PATCH v2 0/3] ARM: dts: Update devicetree of Asrock X570D4U BMC
From: Renze Nicolai @ 2024-04-03 13:28 UTC (permalink / raw)
To: renze, linux-arm-kernel, devicetree, linux-kernel, linux-aspeed,
arnd, olof, soc, robh+dt, krzysztof.kozlowski+dt, joel, andrew
These patches change the GPIO table, ADC channel configuration and
I2C bus configuration of the devicetree for the X570D4U BMC as part of
ongoing efforts to support OpenBMC on this platform.
Changes since v1:
- Fixed warnings indicated by checkpatch.pl
- Change commit message of ADC channels commit to match imperative mood
- Restructure GPIO table to better match other ASPEED devices
- Clarify naming scheme better
Best regards,
Renze Nicolai
Renze Nicolai (3):
ARM: dts: Modify GPIO table for Asrock X570D4U BMC
ARM: dts: Disable unused ADC channels for Asrock X570D4U BMC
ARM: dts: Modify I2C bus configuration
.../dts/aspeed/aspeed-bmc-asrock-x570d4u.dts | 131 ++++++++----------
1 file changed, 57 insertions(+), 74 deletions(-)
--
2.44.0
^ permalink raw reply
* [PATCH v2 2/3] ARM: dts: Disable unused ADC channels for Asrock X570D4U BMC
From: Renze Nicolai @ 2024-04-03 13:28 UTC (permalink / raw)
To: renze, linux-arm-kernel, devicetree, linux-kernel, linux-aspeed,
arnd, olof, soc, robh+dt, krzysztof.kozlowski+dt, joel, andrew
In-Reply-To: <20240403133037.37782-1-renze@rnplus.nl>
Additionally adds labels describing the ADC channels.
Signed-off-by: Renze Nicolai <renze@rnplus.nl>
---
.../dts/aspeed/aspeed-bmc-asrock-x570d4u.dts | 29 +++++++++----------
1 file changed, 13 insertions(+), 16 deletions(-)
diff --git a/arch/arm/boot/dts/aspeed/aspeed-bmc-asrock-x570d4u.dts b/arch/arm/boot/dts/aspeed/aspeed-bmc-asrock-x570d4u.dts
index dff69d6ff146..66318ef8caae 100644
--- a/arch/arm/boot/dts/aspeed/aspeed-bmc-asrock-x570d4u.dts
+++ b/arch/arm/boot/dts/aspeed/aspeed-bmc-asrock-x570d4u.dts
@@ -337,20 +337,17 @@ fan@5 {
&adc {
status = "okay";
pinctrl-names = "default";
- pinctrl-0 = <&pinctrl_adc0_default
- &pinctrl_adc1_default
- &pinctrl_adc2_default
- &pinctrl_adc3_default
- &pinctrl_adc4_default
- &pinctrl_adc5_default
- &pinctrl_adc6_default
- &pinctrl_adc7_default
- &pinctrl_adc8_default
- &pinctrl_adc9_default
- &pinctrl_adc10_default
- &pinctrl_adc11_default
- &pinctrl_adc12_default
- &pinctrl_adc13_default
- &pinctrl_adc14_default
- &pinctrl_adc15_default>;
+ pinctrl-0 = <&pinctrl_adc0_default /* 3VSB */
+ &pinctrl_adc1_default /* 5VSB */
+ &pinctrl_adc2_default /* VCPU */
+ &pinctrl_adc3_default /* VSOC */
+ &pinctrl_adc4_default /* VCCM */
+ &pinctrl_adc5_default /* APU-VDDP */
+ &pinctrl_adc6_default /* PM-VDD-CLDO */
+ &pinctrl_adc7_default /* PM-VDDCR-S5 */
+ &pinctrl_adc8_default /* PM-VDDCR */
+ &pinctrl_adc9_default /* VBAT */
+ &pinctrl_adc10_default /* 3V */
+ &pinctrl_adc11_default /* 5V */
+ &pinctrl_adc12_default>; /* 12V */
};
--
2.44.0
^ permalink raw reply related
* Re: [PATCH 3/3] arm64: dts: qcom: msm8996: Add missing UFS host controller reset
From: Manivannan Sadhasivam @ 2024-04-03 13:50 UTC (permalink / raw)
To: Dmitry Baryshkov
Cc: Bjorn Andersson, Konrad Dybcio, Alim Akhtar, Avri Altman,
Bart Van Assche, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Andy Gross, Andy Gross, James E.J. Bottomley, Martin K. Petersen,
linux-arm-msm, linux-scsi, devicetree, linux-kernel
In-Reply-To: <CAA8EJppOPrSfD=VkHm8M0P07=mN_BikT=cGvLe6UFL3OpKVWzA@mail.gmail.com>
On Tue, Apr 02, 2024 at 10:25:32PM +0300, Dmitry Baryshkov wrote:
> On Tue, 2 Apr 2024 at 20:35, Bjorn Andersson <andersson@kernel.org> wrote:
> >
> > On Fri, Feb 09, 2024 at 10:16:25PM +0200, Dmitry Baryshkov wrote:
> > > On Tue, 30 Jan 2024 at 08:55, Manivannan Sadhasivam
> > > <manivannan.sadhasivam@linaro.org> wrote:
> > > >
> > > > On Mon, Jan 29, 2024 at 11:44:15AM +0200, Dmitry Baryshkov wrote:
> > > > > On Mon, 29 Jan 2024 at 09:55, Manivannan Sadhasivam
> > > > > <manivannan.sadhasivam@linaro.org> wrote:
> > > > > >
> > > > > > UFS host controller reset is required for the drivers to properly reset the
> > > > > > controller. Hence, add it.
> > > > > >
> > > > > > Fixes: 57fc67ef0d35 ("arm64: dts: qcom: msm8996: Add ufs related nodes")
> > > > > > Signed-off-by: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>
> > > > >
> > > > > I think I had issues previously when I attempted to reset the
> > > > > controller, but it might be because of the incomplete clocks
> > > > > programming. Let met check whether it works first.
> > > > >
> > > >
> > > > Sure. Please let me know.
> > >
> > > With the clocking fixes in place (I'll send them in a few minutes) and
> > > with this patch the UFS breaks in the following way:
> > >
> >
> > Was this further reviewed/investigated? What would you like me to do
> > with this patch?
>
> I'd say, hold until we can understand what is going on.
>
> Mani, If you have any ideas what can be causing the issue, I'm open to
> testing any ideas or any patches from your side.
> Judging that the UFS breaks after toggling the reset, we might be
> missing some setup steps.
>
I've bombarded the Qcom UFS team with too many questions, but didn't get answer
for all of them. And this is one of those questions.
Let me try to dig further and come back.
- Mani
--
மணிவண்ணன் சதாசிவம்
^ permalink raw reply
* Re: Fixing the devicetree of Rock 5 Model B (and possibly others)
From: Dragan Simic @ 2024-04-03 13:51 UTC (permalink / raw)
To: Pratham Patel
Cc: Saravana Kannan, sebastian.reichel, devicetree, linux-arm-kernel,
linux-rockchip, linux-kernel, regressions, stable
In-Reply-To: <D0A122WK7CB9.33B2TP6UCMJBJ@thefossguy.com>
Hello Pratham,
On 2024-04-03 01:32, Pratham Patel wrote:
> On Tue Apr 2, 2024 at 4:54 AM IST, Saravana Kannan wrote:
>> On Sat, Mar 23, 2024 at 10:10 AM Dragan Simic <dsimic@manjaro.org>
>> wrote:
>> > On 2024-03-23 18:02, Pratham Patel wrote:
>> > > I looked at the patch and tried several things, neither resulted in
>> > > anything that would point me to the core issue. Then I tried this:
>> >
>> > Could you, please, clarify a bit what's the actual issue you're
>> > experiencing on your Rock 5B?
>>
>> Pratham, can you reply to this please? I don't really understand what
>> your issue is for me to be able to help.
>
> I apologize for not replying. Somehow, I did not notice the reply from
> Dragan. :(
No worries, I saw the serial console log file in one of your messages,
which actually provided the answer to my question. :)
> Since this patch was applied, an issue in the Rock 5B's DT has been
> unearthed which now results in the kernel being unable to boot
> properly.
>
> Following is the relevant call trace from the UART capture:
>
> [ 21.595068] Call trace:
> [ 21.595288] smp_call_function_many_cond+0x174/0x5f8
> [ 21.595728] on_each_cpu_cond_mask+0x2c/0x40
> [ 21.596109] cpuidle_register_driver+0x294/0x318
> [ 21.596524] cpuidle_register+0x24/0x100
> [ 21.596875] psci_cpuidle_probe+0x2e4/0x490
> [ 21.597247] platform_probe+0x70/0xd0
> [ 21.597575] really_probe+0x18c/0x3d8
> [ 21.597905] __driver_probe_device+0x84/0x180
> [ 21.598294] driver_probe_device+0x44/0x120
> [ 21.598669] __device_attach_driver+0xc4/0x168
> [ 21.599063] bus_for_each_drv+0x8c/0xf0
> [ 21.599408] __device_attach+0xa4/0x1c0
> [ 21.599748] device_initial_probe+0x1c/0x30
> [ 21.600118] bus_probe_device+0xb4/0xc0
> [ 21.600462] device_add+0x68c/0x888
> [ 21.600775] platform_device_add+0x19c/0x270
> [ 21.601154] platform_device_register_full+0xdc/0x178
> [ 21.601602] psci_idle_init+0xa0/0xc8
> [ 21.601934] do_one_initcall+0x60/0x290
> [ 21.602275] kernel_init_freeable+0x20c/0x3e0
> [ 21.602664] kernel_init+0x2c/0x1f8
> [ 21.602979] ret_from_fork+0x10/0x20
>
>> Also, can you give the output of <debugfs>/devices_deferred for the
>> good vs bad case?
>
> I can't provide you with requested output from the bad case, since the
> kernel never moves past this to an initramfs rescue shell, but
> following
> is the output from v6.8.1 (**with aforementioned patch reverted**).
>
> # cat /sys/kernel/debug/devices_deferred
> fc400000.usb platform: wait for supplier /phy@fed90000/usb3-port
> 1-0022 typec_fusb302: cannot register tcpm port
> fc000000.usb platform: wait for supplier /phy@fed80000/usb3-port
>
> It seems that v6.8.2 works _without needing to revert the patch_. I
> will
> have to look into this sometime this week but it seems like
> a8037ceb8964 (arm64: dts: rockchip: drop rockchip,trcm-sync-tx-only
> from rk3588 i2s)
> seems to be the one that fixed the root issue. I will have to test it
> sometime later this week.
^ permalink raw reply
* Re: [PATCH] dt-bindings: firmware: arm,scmi: Update examples for protocol@13
From: Sudeep Holla @ 2024-04-03 13:53 UTC (permalink / raw)
To: Ulf Hansson
Cc: Cristian Marussi, Rob Herring, Krzysztof Kozlowski, devicetree,
linux-arm-kernel
In-Reply-To: <20240403111106.1110940-1-ulf.hansson@linaro.org>
On Wed, Apr 03, 2024 at 01:11:06PM +0200, Ulf Hansson wrote:
> Recently we extended the binding for protocol@13 to allow it to be modelled
> as a generic performance domain. In a way to promote using the new binding,
> let's update the examples.
>
Does it make sense to keep one DVFS example with #clock-cells until we
mark it as deprecated ? Otherwise it may be confusing as the binding still
lists. Or leave some comment in the example or something, I am open for
suggestions.
Other than that,
Acked-by: Sudeep Holla <sudeep.holla@arm.com>
--
Regards,
Sudeep
^ permalink raw reply
* Re: [PATCH 1/2] dt-bindings: phy: qmp-ufs: Fix PHY clocks for SC7180
From: Manivannan Sadhasivam @ 2024-04-03 13:53 UTC (permalink / raw)
To: Danila Tikhonov
Cc: andersson, konrad.dybcio, vkoul, kishon, robh,
krzysztof.kozlowski+dt, conor+dt, cros-qcom-dts-watchers,
davidwronek, linux-arm-msm, linux-phy, devicetree, linux-kernel
In-Reply-To: <20240401182240.55282-2-danila@jiaxyga.com>
On Mon, Apr 01, 2024 at 09:22:39PM +0300, Danila Tikhonov wrote:
> QMP UFS PHY used in SC7180 requires 3 clocks:
>
> * ref - 19.2MHz reference clock from RPMh
> * ref_aux - Auxiliary reference clock from GCC
> * qref - QREF clock from GCC
>
> This change obviously breaks the ABI, but it is inevitable since the
> clock topology needs to be accurately described in the binding.
>
> Signed-off-by: Danila Tikhonov <danila@jiaxyga.com>
Reviewed-by: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>
- Mani
> ---
> .../devicetree/bindings/phy/qcom,sc8280xp-qmp-ufs-phy.yaml | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/Documentation/devicetree/bindings/phy/qcom,sc8280xp-qmp-ufs-phy.yaml b/Documentation/devicetree/bindings/phy/qcom,sc8280xp-qmp-ufs-phy.yaml
> index 91a6cc38ff7f..a79fde9a8cdf 100644
> --- a/Documentation/devicetree/bindings/phy/qcom,sc8280xp-qmp-ufs-phy.yaml
> +++ b/Documentation/devicetree/bindings/phy/qcom,sc8280xp-qmp-ufs-phy.yaml
> @@ -86,6 +86,7 @@ allOf:
> enum:
> - qcom,msm8998-qmp-ufs-phy
> - qcom,sa8775p-qmp-ufs-phy
> + - qcom,sc7180-qmp-ufs-phy
> - qcom,sc7280-qmp-ufs-phy
> - qcom,sc8180x-qmp-ufs-phy
> - qcom,sc8280xp-qmp-ufs-phy
> --
> 2.44.0
>
--
மணிவண்ணன் சதாசிவம்
^ permalink raw reply
* Re: [PATCH 2/2] arm64: dts: qcom: sc7180: Fix UFS PHY clocks
From: Manivannan Sadhasivam @ 2024-04-03 13:54 UTC (permalink / raw)
To: Danila Tikhonov
Cc: andersson, konrad.dybcio, vkoul, kishon, robh,
krzysztof.kozlowski+dt, conor+dt, cros-qcom-dts-watchers,
davidwronek, linux-arm-msm, linux-phy, devicetree, linux-kernel
In-Reply-To: <20240401182240.55282-3-danila@jiaxyga.com>
On Mon, Apr 01, 2024 at 09:22:40PM +0300, Danila Tikhonov wrote:
> QMP PHY used in SC7180 requires 3 clocks:
>
> * ref - 19.2MHz reference clock from RPMh
> * ref_aux - Auxiliary reference clock from GCC
> * qref - QREF clock from GCC
>
> While at it, let's move 'clocks' property before 'clock-names' to match
> the style used commonly.
>
> Signed-off-by: Danila Tikhonov <danila@jiaxyga.com>
Reviewed-by: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>
- Mani
> ---
> arch/arm64/boot/dts/qcom/sc7180.dtsi | 9 ++++++---
> 1 file changed, 6 insertions(+), 3 deletions(-)
>
> diff --git a/arch/arm64/boot/dts/qcom/sc7180.dtsi b/arch/arm64/boot/dts/qcom/sc7180.dtsi
> index 2b481e20ae38..5c9ec8047f00 100644
> --- a/arch/arm64/boot/dts/qcom/sc7180.dtsi
> +++ b/arch/arm64/boot/dts/qcom/sc7180.dtsi
> @@ -1585,9 +1585,12 @@ ufs_mem_phy: phy@1d87000 {
> compatible = "qcom,sc7180-qmp-ufs-phy",
> "qcom,sm7150-qmp-ufs-phy";
> reg = <0 0x01d87000 0 0x1000>;
> - clocks = <&gcc GCC_UFS_MEM_CLKREF_CLK>,
> - <&gcc GCC_UFS_PHY_PHY_AUX_CLK>;
> - clock-names = "ref", "ref_aux";
> + clocks = <&rpmhcc RPMH_CXO_CLK>,
> + <&gcc GCC_UFS_PHY_PHY_AUX_CLK>,
> + <&gcc GCC_UFS_MEM_CLKREF_CLK>;
> + clock-names = "ref",
> + "ref_aux",
> + "qref";
> power-domains = <&gcc UFS_PHY_GDSC>;
> resets = <&ufs_mem_hc 0>;
> reset-names = "ufsphy";
> --
> 2.44.0
>
--
மணிவண்ணன் சதாசிவம்
^ permalink raw reply
* Re: Fixing the devicetree of Rock 5 Model B (and possibly others)
From: Sebastian Reichel @ 2024-04-03 13:52 UTC (permalink / raw)
To: Pratham Patel
Cc: Saravana Kannan, Dragan Simic, devicetree, linux-arm-kernel,
linux-rockchip, linux-kernel, regressions, stable
In-Reply-To: <D0A2ZL6S8UG6.2BQKIBQWYB36D@thefossguy.com>
[-- Attachment #1: Type: text/plain, Size: 1529 bytes --]
Hi,
On Wed, Apr 03, 2024 at 01:03:07AM +0000, Pratham Patel wrote:
> > > > Also, can you give the output of <debugfs>/devices_deferred for the
> > > > good vs bad case?
> > >
> > > I can't provide you with requested output from the bad case, since the
> > > kernel never moves past this to an initramfs rescue shell, but following
> > > is the output from v6.8.1 (**with aforementioned patch reverted**).
> > >
> > > # cat /sys/kernel/debug/devices_deferred
> > > fc400000.usb platform: wait for supplier /phy@fed90000/usb3-port
> > > 1-0022 typec_fusb302: cannot register tcpm port
> > > fc000000.usb platform: wait for supplier /phy@fed80000/usb3-port
> > >
> > > It seems that v6.8.2 works _without needing to revert the patch_. I will
> > > have to look into this sometime this week but it seems like
> > > a8037ceb8964 (arm64: dts: rockchip: drop rockchip,trcm-sync-tx-only from rk3588 i2s)
> > > seems to be the one that fixed the root issue. I will have to test it
> > > sometime later this week.
> >
> > Ok, once you find the patch that fixes things, let me know too.
>
> Will do!
FWIW the v6.8.1 kernel referenced above is definitely patched, since
upstream's Rock 5B DT does neither describe fusb302, nor the USB
port it is connected to.
We have a few Rock 5B in Kernel CI and upstream boots perfectly
fine:
https://lava.collabora.dev/scheduler/device_type/rk3588-rock-5b
So it could be one of your downstream patches, which is introducing
this problem.
-- Sebastian
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply
* Re: [PATCH v2 3/3] arm64: dts: qcom: msm8998: set qcom,no-msa-ready-indicator for wifi
From: Dmitry Baryshkov @ 2024-04-03 14:12 UTC (permalink / raw)
To: Marc Gonzalez
Cc: Konrad Dybcio, Krzysztof Kozlowski, Kalle Valo, Jeff Johnson,
ath10k, wireless, DT, MSM, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, Pierre-Hugues Husson, Arnaud Vrac, Bjorn Andersson,
Jami Kettunen, Jeffrey Hugo
In-Reply-To: <91031ed0-104a-4752-8b1e-0dbe15ebf201@freebox.fr>
On Wed, 3 Apr 2024 at 16:05, Marc Gonzalez <mgonzalez@freebox.fr> wrote:
>
> On 02/04/2024 17:55, Dmitry Baryshkov wrote:
>
> > On Tue, 2 Apr 2024 at 18:31, Marc Gonzalez wrote:
> >
> >> So, if I understand correctly, I take this to mean that I should:
> >>
> >> 1) DELETE the qcom,no-msa-ready-indicator boolean property,
> >> 2) ADD a "qcom,msm8998-wifi" (name OK?) compatible,
> >
> > I'd say, this is not correct. There is no "msm8998-wifi".
>
> Can you explain what you mean by:
> 'There is no "msm8998-wifi".' ?
>
> Do you mean that: this compatible string does not exist?
> (I am proposing that it be created.)
>
> Or do you mean that: "msm8998-wifi" is a bad name?
I mean, it is qcom,wcn3990-wifi, because the chip is wcn3990.
> And these strings in ath11k:
>
> Documentation/devicetree/bindings/net/wireless/qcom,ath11k.yaml: - qcom,ipq8074-wifi
> Documentation/devicetree/bindings/net/wireless/qcom,ath11k.yaml: - qcom,ipq6018-wifi
> Documentation/devicetree/bindings/net/wireless/qcom,ath11k.yaml: - qcom,wcn6750-wifi
> Documentation/devicetree/bindings/net/wireless/qcom,ath11k.yaml: - qcom,ipq5018-wifi
I must admit, I don't know the IPQ product naming (it well might be
that it is both the name of the SoC and the WiFI SoC).
>
> > I'd say, we should take a step back and actually verify how this was
> > handled in the vendor kernel.
>
> In our commercial product, we use the ath10k driver in the vendor kernel (v4.4 r38-rel).
I see.
> It looks like Jeff has already performed the code analysis
> wrt vendor vs mainline (including user-space tools).
From his message it looks like we are expected to get MSA READY even on msm8998.
--
With best wishes
Dmitry
^ permalink raw reply
* Re: [PATCH v2 3/3] arm64: dts: qcom: msm8998: set qcom,no-msa-ready-indicator for wifi
From: Krzysztof Kozlowski @ 2024-04-03 14:14 UTC (permalink / raw)
To: Marc Gonzalez, Dmitry Baryshkov
Cc: Konrad Dybcio, Kalle Valo, Jeff Johnson, ath10k, wireless, DT,
MSM, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Pierre-Hugues Husson, Arnaud Vrac, Bjorn Andersson, Jami Kettunen,
Jeffrey Hugo
In-Reply-To: <91031ed0-104a-4752-8b1e-0dbe15ebf201@freebox.fr>
On 03/04/2024 15:05, Marc Gonzalez wrote:
> On 02/04/2024 17:55, Dmitry Baryshkov wrote:
>
>> On Tue, 2 Apr 2024 at 18:31, Marc Gonzalez wrote:
>>
>>> So, if I understand correctly, I take this to mean that I should:
>>>
>>> 1) DELETE the qcom,no-msa-ready-indicator boolean property,
>>> 2) ADD a "qcom,msm8998-wifi" (name OK?) compatible,
>>
>> I'd say, this is not correct. There is no "msm8998-wifi".
>
> Can you explain what you mean by:
> 'There is no "msm8998-wifi".' ?
>
> Do you mean that: this compatible string does not exist?
> (I am proposing that it be created.)
>
> Or do you mean that: "msm8998-wifi" is a bad name?
msm8998 SoC does not have WiFi.
>
>
> I meant to mimic these strings for various sub-blocks:
>
> arch/arm64/boot/dts/qcom/msm8998.dtsi: compatible = "qcom,msm8998-rpm-proc", "qcom,rpm-proc";
> arch/arm64/boot/dts/qcom/msm8998.dtsi: compatible = "qcom,msm8998-rpmpd";
These are all parts of SoC. What you are adding is not part of original
SoC, I think.
Best regards,
Krzysztof
^ 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