From bd6e61df4b2e69985daa312ce28b6af629b30870 Mon Sep 17 00:00:00 2001 From: Damien Le Moal Date: Thu, 17 Oct 2024 10:58:47 +0900 Subject: [PATCH 01/16] PCI: rockchip-ep: Improve link training MIME-Version: 1.0 Content-Type: text/plain; charset=utf8 Content-Transfer-Encoding: 8bit The Rockchip RK3399 TRM V1.3 Part2, Section 17.5.8.1.2, step 7, describes 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. Link: https://lore.kernel.org/r/20241017015849.190271-13-dlemoal@kernel.org Signed-off-by: Damien Le Moal [kwilczynski: update log messages to make them consistent] Signed-off-by: Krzysztof Wilczyński Signed-off-by: Bjorn Helgaas --- drivers/pci/controller/pcie-rockchip-ep.c | 82 ++++++++++++++++++++++- drivers/pci/controller/pcie-rockchip.h | 11 +++ 2 files changed, 92 insertions(+), 1 deletion(-) diff --git a/drivers/pci/controller/pcie-rockchip-ep.c b/drivers/pci/controller/pcie-rockchip-ep.c index 13755e4ed59d..b4d405e4c954 100644 --- a/drivers/pci/controller/pcie-rockchip-ep.c +++ b/drivers/pci/controller/pcie-rockchip-ep.c @@ -10,12 +10,14 @@ #include #include +#include #include #include #include #include #include #include +#include #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, @@ -473,6 +476,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; } @@ -481,6 +486,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 | @@ -488,8 +495,80 @@ 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: if gen2 speed was requested and we are not + * at gen2 speed yet, retrain again for gen2. + */ + 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 (negotiated 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); + + /* 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, @@ -647,6 +726,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..24796176f658 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 (500 * 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.50.1 From a7137cbf6bd53a9f9c40c64fc8b12b88289b3d4a Mon Sep 17 00:00:00 2001 From: Damien Le Moal Date: Thu, 17 Oct 2024 10:58:48 +0900 Subject: [PATCH 02/16] PCI: rockchip-ep: Handle PERST# signal in EP mode MIME-Version: 1.0 Content-Type: text/plain; charset=utf8 Content-Transfer-Encoding: 8bit Currently, the Rockchip PCIe endpoint controller driver does not handle the PERST# signal, which prevents detecting when link training should actually be started or if the host resets the device. This however can be supported using the controller reset_gpios property set as an input GPIO for endpoint mode. Modify the Rockchip PCI endpoint controller driver to get the reset_gpio and its associated interrupt which is serviced using a threaded IRQ with the function rockchip_pcie_ep_perst_irq_thread() as handler. This handler function notifies a link down event corresponding to the RC side asserting the PERST# signal using pci_epc_linkdown() when the gpio is high. Once the gpio value goes down, corresponding to the RC de-asserting the PERST# signal, link training is started. The polarity of the gpio interrupt trigger is changed from high to low after the RC asserted PERST#, and conversely changed from low to high after the RC de-asserts PERST#. Also, given that the host mode controller and the endpoint mode controller use two different property names for the same PERST# signal (ep_gpios property and reset_gpios property respectively), for clarity, rename the ep_gpio field of struct rockchip_pcie to perst_gpio. Link: https://lore.kernel.org/r/20241017015849.190271-14-dlemoal@kernel.org Signed-off-by: Damien Le Moal [kwilczynski: make log messages consistent, add missing include] Signed-off-by: Krzysztof Wilczyński Signed-off-by: Bjorn Helgaas --- drivers/pci/controller/pcie-rockchip-ep.c | 133 +++++++++++++++++++- drivers/pci/controller/pcie-rockchip-host.c | 4 +- drivers/pci/controller/pcie-rockchip.c | 16 +-- drivers/pci/controller/pcie-rockchip.h | 2 +- 4 files changed, 141 insertions(+), 14 deletions(-) diff --git a/drivers/pci/controller/pcie-rockchip-ep.c b/drivers/pci/controller/pcie-rockchip-ep.c index b4d405e4c954..1064b7b06cef 100644 --- a/drivers/pci/controller/pcie-rockchip-ep.c +++ b/drivers/pci/controller/pcie-rockchip-ep.c @@ -10,8 +10,10 @@ #include #include +#include #include #include +#include #include #include #include @@ -50,6 +52,9 @@ struct rockchip_pcie_ep { u64 irq_pci_addr; u8 irq_pci_fn; u8 irq_pending; + int perst_irq; + bool perst_asserted; + bool link_up; struct delayed_work link_training; }; @@ -470,13 +475,17 @@ static int rockchip_pcie_ep_start(struct pci_epc *epc) rockchip_pcie_write(rockchip, cfg, PCIE_CORE_PHY_FUNC_CFG); + if (rockchip->perst_gpio) + enable_irq(ep->perst_irq); + /* Enable configuration and start link training */ rockchip_pcie_write(rockchip, PCIE_CLIENT_LINK_TRAIN_ENABLE | PCIE_CLIENT_CONF_ENABLE, PCIE_CLIENT_CONFIG); - schedule_delayed_work(&ep->link_training, 0); + if (!rockchip->perst_gpio) + schedule_delayed_work(&ep->link_training, 0); return 0; } @@ -486,6 +495,11 @@ 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; + if (rockchip->perst_gpio) { + ep->perst_asserted = true; + disable_irq(ep->perst_irq); + } + cancel_delayed_work_sync(&ep->link_training); /* Stop link training and disable configuration */ @@ -551,6 +565,13 @@ static void rockchip_pcie_ep_link_training(struct work_struct *work) if (!rockchip_pcie_ep_link_up(rockchip)) goto again; + /* + * If PERST# was asserted while polling the link, do not notify + * the function. + */ + if (ep->perst_asserted) + return; + val = rockchip_pcie_read(rockchip, PCIE_CLIENT_BASIC_STATUS0); dev_info(dev, "link up (negotiated speed: %sGT/s, width: x%lu)\n", @@ -560,6 +581,7 @@ static void rockchip_pcie_ep_link_training(struct work_struct *work) /* Notify the function */ pci_epc_linkup(ep->epc); + ep->link_up = true; return; @@ -567,6 +589,103 @@ again: schedule_delayed_work(&ep->link_training, msecs_to_jiffies(5)); } +static void rockchip_pcie_ep_perst_assert(struct rockchip_pcie_ep *ep) +{ + struct rockchip_pcie *rockchip = &ep->rockchip; + + dev_dbg(rockchip->dev, "PERST# asserted, link down\n"); + + if (ep->perst_asserted) + return; + + ep->perst_asserted = true; + + cancel_delayed_work_sync(&ep->link_training); + + if (ep->link_up) { + pci_epc_linkdown(ep->epc); + ep->link_up = false; + } +} + +static void rockchip_pcie_ep_perst_deassert(struct rockchip_pcie_ep *ep) +{ + struct rockchip_pcie *rockchip = &ep->rockchip; + + dev_dbg(rockchip->dev, "PERST# de-asserted, starting link training\n"); + + if (!ep->perst_asserted) + return; + + ep->perst_asserted = false; + + /* Enable link re-training */ + rockchip_pcie_ep_retrain_link(rockchip); + + /* Start link training */ + schedule_delayed_work(&ep->link_training, 0); +} + +static irqreturn_t rockchip_pcie_ep_perst_irq_thread(int irq, void *data) +{ + struct pci_epc *epc = data; + struct rockchip_pcie_ep *ep = epc_get_drvdata(epc); + struct rockchip_pcie *rockchip = &ep->rockchip; + u32 perst = gpiod_get_value(rockchip->perst_gpio); + + if (perst) + rockchip_pcie_ep_perst_assert(ep); + else + rockchip_pcie_ep_perst_deassert(ep); + + irq_set_irq_type(ep->perst_irq, + (perst ? IRQF_TRIGGER_HIGH : IRQF_TRIGGER_LOW)); + + return IRQ_HANDLED; +} + +static int rockchip_pcie_ep_setup_irq(struct pci_epc *epc) +{ + struct rockchip_pcie_ep *ep = epc_get_drvdata(epc); + struct rockchip_pcie *rockchip = &ep->rockchip; + struct device *dev = rockchip->dev; + int ret; + + if (!rockchip->perst_gpio) + return 0; + + /* PCIe reset interrupt */ + ep->perst_irq = gpiod_to_irq(rockchip->perst_gpio); + if (ep->perst_irq < 0) { + dev_err(dev, + "failed to get IRQ for PERST# GPIO: %d\n", + ep->perst_irq); + + return ep->perst_irq; + } + + /* + * The perst_gpio is active low, so when it is inactive on start, it + * is high and will trigger the perst_irq handler. So treat this initial + * IRQ as a dummy one by faking the host asserting PERST#. + */ + ep->perst_asserted = true; + irq_set_status_flags(ep->perst_irq, IRQ_NOAUTOEN); + ret = devm_request_threaded_irq(dev, ep->perst_irq, NULL, + rockchip_pcie_ep_perst_irq_thread, + IRQF_TRIGGER_HIGH | IRQF_ONESHOT, + "pcie-ep-perst", epc); + if (ret) { + dev_err(dev, + "failed to request IRQ for PERST# GPIO: %d\n", + ret); + + return ret; + } + + return 0; +} + static const struct pci_epc_features rockchip_pcie_epc_features = { .linkup_notifier = true, .msi_capable = true, @@ -730,7 +849,7 @@ static int rockchip_pcie_ep_probe(struct platform_device *pdev) epc = devm_pci_epc_create(dev, &rockchip_pcie_epc_ops); if (IS_ERR(epc)) { - dev_err(dev, "failed to create epc device\n"); + dev_err(dev, "failed to create EPC device\n"); return PTR_ERR(epc); } @@ -760,11 +879,17 @@ static int rockchip_pcie_ep_probe(struct platform_device *pdev) pci_epc_init_notify(epc); + err = rockchip_pcie_ep_setup_irq(epc); + if (err < 0) + goto err_uninit_port; + return 0; -err_exit_ob_mem: - rockchip_pcie_ep_exit_ob_mem(ep); +err_uninit_port: + rockchip_pcie_deinit_phys(rockchip); err_disable_clocks: rockchip_pcie_disable_clocks(rockchip); +err_exit_ob_mem: + rockchip_pcie_ep_exit_ob_mem(ep); return err; } diff --git a/drivers/pci/controller/pcie-rockchip-host.c b/drivers/pci/controller/pcie-rockchip-host.c index cbec71114825..7471d9fd18bc 100644 --- a/drivers/pci/controller/pcie-rockchip-host.c +++ b/drivers/pci/controller/pcie-rockchip-host.c @@ -294,7 +294,7 @@ static int rockchip_pcie_host_init_port(struct rockchip_pcie *rockchip) int err, i = MAX_LANE_NUM; u32 status; - gpiod_set_value_cansleep(rockchip->ep_gpio, 0); + gpiod_set_value_cansleep(rockchip->perst_gpio, 0); err = rockchip_pcie_init_port(rockchip); if (err) @@ -323,7 +323,7 @@ static int rockchip_pcie_host_init_port(struct rockchip_pcie *rockchip) PCIE_CLIENT_CONFIG); msleep(PCIE_T_PVPERL_MS); - gpiod_set_value_cansleep(rockchip->ep_gpio, 1); + gpiod_set_value_cansleep(rockchip->perst_gpio, 1); msleep(PCIE_T_RRS_READY_MS); diff --git a/drivers/pci/controller/pcie-rockchip.c b/drivers/pci/controller/pcie-rockchip.c index 154e78819e6e..b9ade7632e11 100644 --- a/drivers/pci/controller/pcie-rockchip.c +++ b/drivers/pci/controller/pcie-rockchip.c @@ -119,13 +119,15 @@ int rockchip_pcie_parse_dt(struct rockchip_pcie *rockchip) return PTR_ERR(rockchip->aclk_rst); } - if (rockchip->is_rc) { - rockchip->ep_gpio = devm_gpiod_get_optional(dev, "ep", - GPIOD_OUT_LOW); - if (IS_ERR(rockchip->ep_gpio)) - return dev_err_probe(dev, PTR_ERR(rockchip->ep_gpio), - "failed to get ep GPIO\n"); - } + if (rockchip->is_rc) + rockchip->perst_gpio = devm_gpiod_get_optional(dev, "ep", + GPIOD_OUT_LOW); + else + rockchip->perst_gpio = devm_gpiod_get_optional(dev, "reset", + GPIOD_IN); + if (IS_ERR(rockchip->perst_gpio)) + return dev_err_probe(dev, PTR_ERR(rockchip->perst_gpio), + "failed to get PERST# GPIO\n"); rockchip->aclk_pcie = devm_clk_get(dev, "aclk"); if (IS_ERR(rockchip->aclk_pcie)) { diff --git a/drivers/pci/controller/pcie-rockchip.h b/drivers/pci/controller/pcie-rockchip.h index 24796176f658..a51b087ce878 100644 --- a/drivers/pci/controller/pcie-rockchip.h +++ b/drivers/pci/controller/pcie-rockchip.h @@ -329,7 +329,7 @@ struct rockchip_pcie { struct regulator *vpcie3v3; /* 3.3V power supply */ struct regulator *vpcie1v8; /* 1.8V power supply */ struct regulator *vpcie0v9; /* 0.9V power supply */ - struct gpio_desc *ep_gpio; + struct gpio_desc *perst_gpio; u32 lanes; u8 lanes_map; int link_gen; -- 2.50.1 From 43a43faf5376114161aa684834d24e06da596287 Mon Sep 17 00:00:00 2001 From: Linus Torvalds Date: Fri, 22 Nov 2024 11:18:25 -0800 Subject: [PATCH 03/16] futex: improve user space accesses Josh Poimboeuf reports that he got a "will-it-scale.per_process_ops 1.9% improvement" report for his patch that changed __get_user() to use pointer masking instead of the explicit speculation barrier. However, that patch doesn't actually work in the general case, because some (very bad) architecture-specific code actually depends on __get_user() also working on kernel addresses. A profile showed that the offending __get_user() was the futex code, which really should be fixed up to not use that horrid legacy case. Rewrite futex_get_value_locked() to use the modern user acccess helpers, and inline it so that the compiler not only avoids the function call for a few instructions, but can do CSE on the address masking. It also turns out the x86 futex functions have unnecessary barriers in other places, so let's fix those up too. Link: https://lore.kernel.org/all/20241115230653.hfvzyf3aqqntgp63@jpoimboe/ Reported-by: Josh Poimboeuf Signed-off-by: Linus Torvalds --- arch/x86/include/asm/futex.h | 8 +++-- kernel/futex/core.c | 22 -------------- kernel/futex/futex.h | 59 ++++++++++++++++++++++++++++++++++-- 3 files changed, 63 insertions(+), 26 deletions(-) diff --git a/arch/x86/include/asm/futex.h b/arch/x86/include/asm/futex.h index 99d345b686fa..6e2458088800 100644 --- a/arch/x86/include/asm/futex.h +++ b/arch/x86/include/asm/futex.h @@ -48,7 +48,9 @@ do { \ static __always_inline int arch_futex_atomic_op_inuser(int op, int oparg, int *oval, u32 __user *uaddr) { - if (!user_access_begin(uaddr, sizeof(u32))) + if (can_do_masked_user_access()) + uaddr = masked_user_access_begin(uaddr); + else if (!user_access_begin(uaddr, sizeof(u32))) return -EFAULT; switch (op) { @@ -84,7 +86,9 @@ static inline int futex_atomic_cmpxchg_inatomic(u32 *uval, u32 __user *uaddr, { int ret = 0; - if (!user_access_begin(uaddr, sizeof(u32))) + if (can_do_masked_user_access()) + uaddr = masked_user_access_begin(uaddr); + else if (!user_access_begin(uaddr, sizeof(u32))) return -EFAULT; asm volatile("\n" "1:\t" LOCK_PREFIX "cmpxchgl %3, %2\n" diff --git a/kernel/futex/core.c b/kernel/futex/core.c index 6de57246760e..ebdd76b4ecbb 100644 --- a/kernel/futex/core.c +++ b/kernel/futex/core.c @@ -451,28 +451,6 @@ struct futex_q *futex_top_waiter(struct futex_hash_bucket *hb, union futex_key * return NULL; } -int futex_cmpxchg_value_locked(u32 *curval, u32 __user *uaddr, u32 uval, u32 newval) -{ - int ret; - - pagefault_disable(); - ret = futex_atomic_cmpxchg_inatomic(curval, uaddr, uval, newval); - pagefault_enable(); - - return ret; -} - -int futex_get_value_locked(u32 *dest, u32 __user *from) -{ - int ret; - - pagefault_disable(); - ret = __get_user(*dest, from); - pagefault_enable(); - - return ret ? -EFAULT : 0; -} - /** * wait_for_owner_exiting - Block until the owner has exited * @ret: owner's current futex lock status diff --git a/kernel/futex/futex.h b/kernel/futex/futex.h index 8b195d06f4e8..618ce1fe870e 100644 --- a/kernel/futex/futex.h +++ b/kernel/futex/futex.h @@ -6,6 +6,7 @@ #include #include #include +#include #ifdef CONFIG_PREEMPT_RT #include @@ -225,10 +226,64 @@ extern bool __futex_wake_mark(struct futex_q *q); extern void futex_wake_mark(struct wake_q_head *wake_q, struct futex_q *q); extern int fault_in_user_writeable(u32 __user *uaddr); -extern int futex_cmpxchg_value_locked(u32 *curval, u32 __user *uaddr, u32 uval, u32 newval); -extern int futex_get_value_locked(u32 *dest, u32 __user *from); extern struct futex_q *futex_top_waiter(struct futex_hash_bucket *hb, union futex_key *key); +static inline int futex_cmpxchg_value_locked(u32 *curval, u32 __user *uaddr, u32 uval, u32 newval) +{ + int ret; + + pagefault_disable(); + ret = futex_atomic_cmpxchg_inatomic(curval, uaddr, uval, newval); + pagefault_enable(); + + return ret; +} + +/* + * This does a plain atomic user space read, and the user pointer has + * already been verified earlier by get_futex_key() to be both aligned + * and actually in user space, just like futex_atomic_cmpxchg_inatomic(). + * + * We still want to avoid any speculation, and while __get_user() is + * the traditional model for this, it's actually slower than doing + * this manually these days. + * + * We could just have a per-architecture special function for it, + * the same way we do futex_atomic_cmpxchg_inatomic(), but rather + * than force everybody to do that, write it out long-hand using + * the low-level user-access infrastructure. + * + * This looks a bit overkill, but generally just results in a couple + * of instructions. + */ +static __always_inline int futex_read_inatomic(u32 *dest, u32 __user *from) +{ + u32 val; + + if (can_do_masked_user_access()) + from = masked_user_access_begin(from); + else if (!user_read_access_begin(from, sizeof(*from))) + return -EFAULT; + unsafe_get_user(val, from, Efault); + user_access_end(); + *dest = val; + return 0; +Efault: + user_access_end(); + return -EFAULT; +} + +static inline int futex_get_value_locked(u32 *dest, u32 __user *from) +{ + int ret; + + pagefault_disable(); + ret = futex_read_inatomic(dest, from); + pagefault_enable(); + + return ret; +} + extern void __futex_unqueue(struct futex_q *q); extern void __futex_queue(struct futex_q *q, struct futex_hash_bucket *hb); extern int futex_unqueue(struct futex_q *q); -- 2.50.1 From 573f45a9f9a47fed4c7957609689b772121b33d7 Mon Sep 17 00:00:00 2001 From: David Laight Date: Sun, 24 Nov 2024 15:39:00 +0000 Subject: [PATCH 04/16] x86: fix off-by-one in access_ok() When the size isn't a small constant, __access_ok() will call valid_user_address() with the address after the last byte of the user buffer. It is valid for a buffer to end with the last valid user address so valid_user_address() must allow accesses to the base of the guard page. [ This introduces an off-by-one in the other direction for the plain non-sized accesses, but since we have that guard region that is a whole page, those checks "allowing" accesses to that guard region don't really matter. The access will fault anyway, whether to the guard page or if the address has been masked to all ones - Linus ] Fixes: 86e6b1547b3d0 ("x86: fix user address masking non-canonical speculation issue") Signed-off-by: David Laight Signed-off-by: Linus Torvalds --- arch/x86/kernel/cpu/common.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/arch/x86/kernel/cpu/common.c b/arch/x86/kernel/cpu/common.c index 06a516f6795b..ca327cfa42ae 100644 --- a/arch/x86/kernel/cpu/common.c +++ b/arch/x86/kernel/cpu/common.c @@ -2389,12 +2389,12 @@ void __init arch_cpu_finalize_init(void) alternative_instructions(); if (IS_ENABLED(CONFIG_X86_64)) { - unsigned long USER_PTR_MAX = TASK_SIZE_MAX-1; + unsigned long USER_PTR_MAX = TASK_SIZE_MAX; /* * Enable this when LAM is gated on LASS support if (cpu_feature_enabled(X86_FEATURE_LAM)) - USER_PTR_MAX = (1ul << 63) - PAGE_SIZE - 1; + USER_PTR_MAX = (1ul << 63) - PAGE_SIZE; */ runtime_const_init(ptr, USER_PTR_MAX); -- 2.50.1 From 222974c6ec9d901f7ad13bbe6e505ec1f1d822d4 Mon Sep 17 00:00:00 2001 From: Linus Torvalds Date: Mon, 25 Nov 2024 18:45:33 -0800 Subject: [PATCH 05/16] iommu: remove stale declaration left over by a merge conflict The merge commit ae3325f752ef ("Merge branches 'arm/smmu', 'mediatek', 's390', 'ti/omap', 'riscv' and 'core' into next") left a stale declaration of 'iommu_present()' even though the 'core' branch that was merged had removed the function (and the declaration). Remove it for real. Reported-by: Dmitry Safonov <0x7f454c46@gmail.com> Cc: Joerg Roedel Signed-off-by: Linus Torvalds --- include/linux/iommu.h | 1 - 1 file changed, 1 deletion(-) diff --git a/include/linux/iommu.h b/include/linux/iommu.h index d6aaaec3caf4..301e97d745c1 100644 --- a/include/linux/iommu.h +++ b/include/linux/iommu.h @@ -841,7 +841,6 @@ static inline void iommu_iotlb_gather_init(struct iommu_iotlb_gather *gather) }; } -extern bool iommu_present(const struct bus_type *bus); extern bool device_iommu_capable(struct device *dev, enum iommu_cap cap); extern bool iommu_group_has_isolated_msi(struct iommu_group *group); struct iommu_domain *iommu_paging_domain_alloc_flags(struct device *dev, unsigned int flags); -- 2.50.1 From 1dc707e647bc919834eff9636c8d00b78c782545 Mon Sep 17 00:00:00 2001 From: Linus Torvalds Date: Tue, 26 Nov 2024 17:54:58 -0800 Subject: [PATCH 06/16] rust: fix up formatting after merge When I merged the rust 'use' imports, I didn't realize that there's an offical preferred idiomatic format - so while it all worked fine, it doesn't match what 'make rustfmt' wants to make it. Fix it up appropriately. Suggested-by: Miguel Ojeda Signed-off-by: Linus Torvalds --- rust/kernel/task.rs | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/rust/kernel/task.rs b/rust/kernel/task.rs index 7a76be583126..07bc22a7645c 100644 --- a/rust/kernel/task.rs +++ b/rust/kernel/task.rs @@ -6,11 +6,15 @@ use crate::{ bindings, + ffi::{c_int, c_long, c_uint}, pid_namespace::PidNamespace, types::{ARef, NotThreadSafe, Opaque}, }; -use crate::ffi::{c_int, c_long, c_uint}; -use core::{cmp::{Eq, PartialEq},ops::Deref, ptr}; +use core::{ + cmp::{Eq, PartialEq}, + ops::Deref, + ptr, +}; /// A sentinel value used for infinite timeouts. pub const MAX_SCHEDULE_TIMEOUT: c_long = c_long::MAX; -- 2.50.1 From 81de291d86b704de1809cfb06672902d003cf3a3 Mon Sep 17 00:00:00 2001 From: Chen-Yu Tsai Date: Wed, 6 Nov 2024 17:33:28 +0800 Subject: [PATCH 07/16] of: dynamic: Add of_changeset_update_prop_string Add a helper function to add string property updates to an OF changeset. This is similar to of_changeset_add_prop_string(), but instead of adding the property (and failing if it exists), it will update the property. This shall be used later in the DT hardware prober. Signed-off-by: Chen-Yu Tsai Reviewed-by: Rob Herring (Arm) Reviewed-by: AngeloGioacchino Del Regno Signed-off-by: Wolfram Sang --- drivers/of/dynamic.c | 44 ++++++++++++++++++++++++++++++++++++++++++++ include/linux/of.h | 4 ++++ 2 files changed, 48 insertions(+) diff --git a/drivers/of/dynamic.c b/drivers/of/dynamic.c index d45a8df61380..0aba760f7577 100644 --- a/drivers/of/dynamic.c +++ b/drivers/of/dynamic.c @@ -1072,3 +1072,47 @@ int of_changeset_add_prop_bool(struct of_changeset *ocs, struct device_node *np, return of_changeset_add_prop_helper(ocs, np, &prop); } EXPORT_SYMBOL_GPL(of_changeset_add_prop_bool); + +static int of_changeset_update_prop_helper(struct of_changeset *ocs, + struct device_node *np, + const struct property *pp) +{ + struct property *new_pp; + int ret; + + new_pp = __of_prop_dup(pp, GFP_KERNEL); + if (!new_pp) + return -ENOMEM; + + ret = of_changeset_update_property(ocs, np, new_pp); + if (ret) + __of_prop_free(new_pp); + + return ret; +} + +/** + * of_changeset_update_prop_string - Add a string property update to a changeset + * + * @ocs: changeset pointer + * @np: device node pointer + * @prop_name: name of the property to be updated + * @str: pointer to null terminated string + * + * Create a string property to be updated and add it to a changeset. + * + * Return: 0 on success, a negative error value in case of an error. + */ +int of_changeset_update_prop_string(struct of_changeset *ocs, + struct device_node *np, + const char *prop_name, const char *str) +{ + struct property prop = { + .name = (char *)prop_name, + .length = strlen(str) + 1, + .value = (void *)str, + }; + + return of_changeset_update_prop_helper(ocs, np, &prop); +} +EXPORT_SYMBOL_GPL(of_changeset_update_prop_string); diff --git a/include/linux/of.h b/include/linux/of.h index 086a60f3b8a6..d0307e3b093d 100644 --- a/include/linux/of.h +++ b/include/linux/of.h @@ -1651,6 +1651,10 @@ static inline int of_changeset_add_prop_u32(struct of_changeset *ocs, return of_changeset_add_prop_u32_array(ocs, np, prop_name, &val, 1); } +int of_changeset_update_prop_string(struct of_changeset *ocs, + struct device_node *np, + const char *prop_name, const char *str); + int of_changeset_add_prop_bool(struct of_changeset *ocs, struct device_node *np, const char *prop_name); -- 2.50.1 From 1fcc67e3a354865775355eafec1fb061a755c971 Mon Sep 17 00:00:00 2001 From: Chen-Yu Tsai Date: Wed, 6 Nov 2024 17:33:29 +0800 Subject: [PATCH 08/16] of: base: Add for_each_child_of_node_with_prefix() There are cases where drivers would go through child device nodes and operate on only the ones whose node name starts with a given prefix. Provide a helper for these users. This will mainly be used in a subsequent patch that implements a hardware component prober for I2C busses. Signed-off-by: Chen-Yu Tsai Reviewed-by: Rob Herring (Arm) Reviewed-by: AngeloGioacchino Del Regno Signed-off-by: Wolfram Sang --- drivers/of/base.c | 35 +++++++++++++++++++++++++++++++++++ include/linux/of.h | 9 +++++++++ 2 files changed, 44 insertions(+) diff --git a/drivers/of/base.c b/drivers/of/base.c index a8b0c42bdc8e..4cba021c89d3 100644 --- a/drivers/of/base.c +++ b/drivers/of/base.c @@ -644,6 +644,41 @@ struct device_node *of_get_next_child(const struct device_node *node, } EXPORT_SYMBOL(of_get_next_child); +/** + * of_get_next_child_with_prefix - Find the next child node with prefix + * @node: parent node + * @prev: previous child of the parent node, or NULL to get first + * + * This function is like of_get_next_child(), except that it automatically + * skips any nodes whose name doesn't have the given prefix. + * + * Return: A node pointer with refcount incremented, use + * of_node_put() on it when done. + */ +struct device_node *of_get_next_child_with_prefix(const struct device_node *node, + struct device_node *prev, + const char *prefix) +{ + struct device_node *next; + unsigned long flags; + + if (!node) + return NULL; + + raw_spin_lock_irqsave(&devtree_lock, flags); + next = prev ? prev->sibling : node->child; + for (; next; next = next->sibling) { + if (!of_node_name_prefix(next, prefix)) + continue; + if (of_node_get(next)) + break; + } + of_node_put(prev); + raw_spin_unlock_irqrestore(&devtree_lock, flags); + return next; +} +EXPORT_SYMBOL(of_get_next_child_with_prefix); + static struct device_node *of_get_next_status_child(const struct device_node *node, struct device_node *prev, bool (*checker)(const struct device_node *)) diff --git a/include/linux/of.h b/include/linux/of.h index d0307e3b093d..f921786cb8ac 100644 --- a/include/linux/of.h +++ b/include/linux/of.h @@ -289,6 +289,9 @@ extern struct device_node *of_get_parent(const struct device_node *node); extern struct device_node *of_get_next_parent(struct device_node *node); extern struct device_node *of_get_next_child(const struct device_node *node, struct device_node *prev); +extern struct device_node *of_get_next_child_with_prefix(const struct device_node *node, + struct device_node *prev, + const char *prefix); extern struct device_node *of_get_next_available_child( const struct device_node *node, struct device_node *prev); extern struct device_node *of_get_next_reserved_child( @@ -1468,6 +1471,12 @@ static inline int of_property_read_s32(const struct device_node *np, child != NULL; \ child = of_get_next_child(parent, child)) +#define for_each_child_of_node_with_prefix(parent, child, prefix) \ + for (struct device_node *child __free(device_node) = \ + of_get_next_child_with_prefix(parent, NULL, prefix); \ + child != NULL; \ + child = of_get_next_child_with_prefix(parent, child, prefix)) + #define for_each_available_child_of_node(parent, child) \ for (child = of_get_next_available_child(parent, NULL); child != NULL; \ child = of_get_next_available_child(parent, child)) -- 2.50.1 From 157ce8f381efe264933e9366db828d845bade3a1 Mon Sep 17 00:00:00 2001 From: Chen-Yu Tsai Date: Wed, 6 Nov 2024 17:33:30 +0800 Subject: [PATCH 09/16] i2c: Introduce OF component probe function Some devices are designed and manufactured with some components having multiple drop-in replacement options. These components are often connected to the mainboard via ribbon cables, having the same signals and pin assignments across all options. These may include the display panel and touchscreen on laptops and tablets, and the trackpad on laptops. Sometimes which component option is used in a particular device can be detected by some firmware provided identifier, other times that information is not available, and the kernel has to try to probe each device. This change attempts to make the "probe each device" case cleaner. The current approach is to have all options added and enabled in the device tree. The kernel would then bind each device and run each driver's probe function. This works, but has been broken before due to the introduction of asynchronous probing, causing multiple instances requesting "shared" resources, such as pinmuxes, GPIO pins, interrupt lines, at the same time, with only one instance succeeding. Work arounds for these include moving the pinmux to the parent I2C controller, using GPIO hogs or pinmux settings to keep the GPIO pins in some fixed configuration, and requesting the interrupt line very late. Such configurations can be seen on the MT8183 Krane Chromebook tablets, and the Qualcomm sc8280xp-based Lenovo Thinkpad 13S. Instead of this delicate dance between drivers and device tree quirks, this change introduces a simple I2C component probe function. For a given class of devices on the same I2C bus, it will go through all of them, doing a simple I2C read transfer and see which one of them responds. It will then enable the device that responds. This requires some minor modifications in the existing device tree. The status for all the device nodes for the component options must be set to "fail-needs-probe". This makes it clear that some mechanism is needed to enable one of them, and also prevents the prober and device drivers running at the same time. Signed-off-by: Chen-Yu Tsai Reviewed-by: Andy Shevchenko Reviewed-by: Douglas Anderson Reviewed-by: AngeloGioacchino Del Regno Signed-off-by: Wolfram Sang --- MAINTAINERS | 8 ++ drivers/i2c/Makefile | 1 + drivers/i2c/i2c-core-of-prober.c | 183 +++++++++++++++++++++++++++++++ include/linux/i2c-of-prober.h | 75 +++++++++++++ 4 files changed, 267 insertions(+) create mode 100644 drivers/i2c/i2c-core-of-prober.c create mode 100644 include/linux/i2c-of-prober.h diff --git a/MAINTAINERS b/MAINTAINERS index 85cdc618a51c..0f837aca327c 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -10702,6 +10702,14 @@ S: Maintained F: Documentation/devicetree/bindings/i2c/marvell,mv64xxx-i2c.yaml F: drivers/i2c/busses/i2c-mv64xxx.c +I2C OF COMPONENT PROBER +M: Chen-Yu Tsai +L: linux-i2c@vger.kernel.org +L: devicetree@vger.kernel.org +S: Maintained +F: drivers/i2c/i2c-core-of-prober.c +F: include/linux-i2c-of-prober.h + I2C OVER PARALLEL PORT M: Jean Delvare L: linux-i2c@vger.kernel.org diff --git a/drivers/i2c/Makefile b/drivers/i2c/Makefile index af95d6e7f004..d27de18de46f 100644 --- a/drivers/i2c/Makefile +++ b/drivers/i2c/Makefile @@ -9,6 +9,7 @@ i2c-core-y := i2c-core-base.o i2c-core-smbus.o i2c-core-$(CONFIG_ACPI) += i2c-core-acpi.o i2c-core-$(CONFIG_I2C_SLAVE) += i2c-core-slave.o i2c-core-$(CONFIG_OF) += i2c-core-of.o +i2c-core-$(CONFIG_OF_DYNAMIC) += i2c-core-of-prober.o obj-$(CONFIG_I2C_SMBUS) += i2c-smbus.o obj-$(CONFIG_I2C_CHARDEV) += i2c-dev.o diff --git a/drivers/i2c/i2c-core-of-prober.c b/drivers/i2c/i2c-core-of-prober.c new file mode 100644 index 000000000000..21f7a4769293 --- /dev/null +++ b/drivers/i2c/i2c-core-of-prober.c @@ -0,0 +1,183 @@ +// SPDX-License-Identifier: GPL-2.0-or-later +/* + * Linux I2C core OF component prober code + * + * Copyright (C) 2024 Google LLC + */ + +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include + +/* + * Some devices, such as Google Hana Chromebooks, are produced by multiple + * vendors each using their preferred components. Such components are all + * in the device tree. Instead of having all of them enabled and having each + * driver separately try and probe its device while fighting over shared + * resources, they can be marked as "fail-needs-probe" and have a prober + * figure out which one is actually used beforehand. + * + * This prober assumes such drop-in parts are on the same I2C bus, have + * non-conflicting addresses, and can be directly probed by seeing which + * address responds. + * + * TODO: + * - Support handling common regulators. + * - Support handling common GPIOs. + * - Support I2C muxes + */ + +static struct device_node *i2c_of_probe_get_i2c_node(struct device *dev, const char *type) +{ + struct device_node *node __free(device_node) = of_find_node_by_name(NULL, type); + if (!node) { + dev_err(dev, "Could not find %s device node\n", type); + return NULL; + } + + struct device_node *i2c_node __free(device_node) = of_get_parent(node); + if (!of_node_name_eq(i2c_node, "i2c")) { + dev_err(dev, "%s device isn't on I2C bus\n", type); + return NULL; + } + + if (!of_device_is_available(i2c_node)) { + dev_err(dev, "I2C controller not available\n"); + return NULL; + } + + return no_free_ptr(i2c_node); +} + +static int i2c_of_probe_enable_node(struct device *dev, struct device_node *node) +{ + int ret; + + dev_dbg(dev, "Enabling %pOF\n", node); + + struct of_changeset *ocs __free(kfree) = kzalloc(sizeof(*ocs), GFP_KERNEL); + if (!ocs) + return -ENOMEM; + + of_changeset_init(ocs); + ret = of_changeset_update_prop_string(ocs, node, "status", "okay"); + if (ret) + return ret; + + ret = of_changeset_apply(ocs); + if (ret) { + /* ocs needs to be explicitly cleaned up before being freed. */ + of_changeset_destroy(ocs); + } else { + /* + * ocs is intentionally kept around as it needs to + * exist as long as the change is applied. + */ + void *ptr __always_unused = no_free_ptr(ocs); + } + + return ret; +} + +static const struct i2c_of_probe_ops i2c_of_probe_dummy_ops; + +/** + * i2c_of_probe_component() - probe for devices of "type" on the same i2c bus + * @dev: Pointer to the &struct device of the caller, only used for dev_printk() messages. + * @cfg: Pointer to the &struct i2c_of_probe_cfg containing callbacks and other options + * for the prober. + * @ctx: Context data for callbacks. + * + * Probe for possible I2C components of the same "type" (&i2c_of_probe_cfg->type) + * on the same I2C bus that have their status marked as "fail-needs-probe". + * + * Assumes that across the entire device tree the only instances of nodes + * with "type" prefixed node names (not including the address portion) are + * the ones that need handling for second source components. In other words, + * if "type" is "touchscreen", then all device nodes named "touchscreen*" + * are the ones that need probing. There cannot be another "touchscreen*" + * node that is already enabled. + * + * Assumes that for each "type" of component, only one actually exists. In + * other words, only one matching and existing device will be enabled. + * + * Context: Process context only. Does non-atomic I2C transfers. + * Should only be used from a driver probe function, as the function + * can return -EPROBE_DEFER if the I2C adapter or other resources + * are unavailable. + * Return: 0 on success or no-op, error code otherwise. + * A no-op can happen when it seems like the device tree already + * has components of the type to be probed already enabled. This + * can happen when the device tree had not been updated to mark + * the status of the to-be-probed components as "fail-needs-probe". + * Or this function was already run with the same parameters and + * succeeded in enabling a component. The latter could happen if + * the user had multiple types of components to probe, and one of + * them down the list caused a deferred probe. This is expected + * behavior. + */ +int i2c_of_probe_component(struct device *dev, const struct i2c_of_probe_cfg *cfg, void *ctx) +{ + const struct i2c_of_probe_ops *ops; + const char *type; + struct i2c_adapter *i2c; + int ret; + + ops = cfg->ops ?: &i2c_of_probe_dummy_ops; + type = cfg->type; + + struct device_node *i2c_node __free(device_node) = i2c_of_probe_get_i2c_node(dev, type); + if (!i2c_node) + return -ENODEV; + + /* + * If any devices of the given "type" are already enabled then this function is a no-op. + * Either the device tree hasn't been modified to work with this probe function, or the + * function had already run before and enabled some component. + */ + for_each_child_of_node_with_prefix(i2c_node, node, type) + if (of_device_is_available(node)) + return 0; + + i2c = of_get_i2c_adapter_by_node(i2c_node); + if (!i2c) + return dev_err_probe(dev, -EPROBE_DEFER, "Couldn't get I2C adapter\n"); + + /* Grab and enable resources */ + ret = 0; + if (ops->enable) + ret = ops->enable(dev, i2c_node, ctx); + if (ret) + goto out_put_i2c_adapter; + + for_each_child_of_node_with_prefix(i2c_node, node, type) { + union i2c_smbus_data data; + u32 addr; + + if (of_property_read_u32(node, "reg", &addr)) + continue; + if (i2c_smbus_xfer(i2c, addr, 0, I2C_SMBUS_READ, 0, I2C_SMBUS_BYTE, &data) < 0) + continue; + + /* Found a device that is responding */ + if (ops->cleanup_early) + ops->cleanup_early(dev, ctx); + ret = i2c_of_probe_enable_node(dev, node); + break; + } + + if (ops->cleanup) + ops->cleanup(dev, ctx); +out_put_i2c_adapter: + i2c_put_adapter(i2c); + + return ret; +} +EXPORT_SYMBOL_NS_GPL(i2c_of_probe_component, I2C_OF_PROBER); diff --git a/include/linux/i2c-of-prober.h b/include/linux/i2c-of-prober.h new file mode 100644 index 000000000000..e7e052ac9e48 --- /dev/null +++ b/include/linux/i2c-of-prober.h @@ -0,0 +1,75 @@ +/* SPDX-License-Identifier: GPL-2.0-or-later */ +/* + * Definitions for the Linux I2C OF component prober + * + * Copyright (C) 2024 Google LLC + */ + +#ifndef _LINUX_I2C_OF_PROBER_H +#define _LINUX_I2C_OF_PROBER_H + +#include + +struct device; +struct device_node; + +/** + * struct i2c_of_probe_ops - I2C OF component prober callbacks + * + * A set of callbacks to be used by i2c_of_probe_component(). + * + * All callbacks are optional. Callbacks are called only once per run, and are + * used in the order they are defined in this structure. + * + * All callbacks that have return values shall return %0 on success, + * or a negative error number on failure. + * + * The @dev parameter passed to the callbacks is the same as @dev passed to + * i2c_of_probe_component(). It should only be used for dev_printk() calls + * and nothing else, especially not managed device resource (devres) APIs. + */ +struct i2c_of_probe_ops { + /** + * @enable: Retrieve and enable resources so that the components respond to probes. + * + * It is OK for this callback to return -EPROBE_DEFER since the intended use includes + * retrieving resources and enables them. Resources should be reverted to their initial + * state and released before returning if this fails. + */ + int (*enable)(struct device *dev, struct device_node *bus_node, void *data); + + /** + * @cleanup_early: Release exclusive resources prior to calling probe() on a + * detected component. + * + * Only called if a matching component is actually found. If none are found, + * resources that would have been released in this callback should be released in + * @free_resourcs_late instead. + */ + void (*cleanup_early)(struct device *dev, void *data); + + /** + * @cleanup: Opposite of @enable to balance refcounts and free resources after probing. + * + * Should check if resources were already freed by @cleanup_early. + */ + void (*cleanup)(struct device *dev, void *data); +}; + +/** + * struct i2c_of_probe_cfg - I2C OF component prober configuration + * @ops: Callbacks for the prober to use. + * @type: A string to match the device node name prefix to probe for. + */ +struct i2c_of_probe_cfg { + const struct i2c_of_probe_ops *ops; + const char *type; +}; + +#if IS_ENABLED(CONFIG_OF_DYNAMIC) + +int i2c_of_probe_component(struct device *dev, const struct i2c_of_probe_cfg *cfg, void *ctx); + +#endif /* IS_ENABLED(CONFIG_OF_DYNAMIC) */ + +#endif /* _LINUX_I2C_OF_PROBER_H */ -- 2.50.1 From 897261149d255d03fc90bec6782e3835cacbfdde Mon Sep 17 00:00:00 2001 From: Chen-Yu Tsai Date: Wed, 6 Nov 2024 17:33:31 +0800 Subject: [PATCH 10/16] i2c: of-prober: Add simple helpers for regulator support Add helpers to do regulator management for the I2C OF component prober. Components that the prober intends to probe likely require their regulator supplies be enabled, and GPIOs be toggled to enable them or bring them out of reset before they will respond to probe attempts. GPIOs will be handled in the next patch. The assumption is that the same class of components to be probed are always connected in the same fashion with the same regulator supply and GPIO. The names may vary due to binding differences, but the physical layout does not change. This set of helpers supports at most one regulator supply. The user must specify the node from which the supply is retrieved. The supply name and the amount of time to wait after the supply is enabled are also given by the user. Signed-off-by: Chen-Yu Tsai Reviewed-by: Douglas Anderson Reviewed-by: Andy Shevchenko Reviewed-by: AngeloGioacchino Del Regno Signed-off-by: Wolfram Sang --- drivers/i2c/i2c-core-of-prober.c | 138 ++++++++++++++++++++++++++++++- include/linux/i2c-of-prober.h | 44 ++++++++++ 2 files changed, 181 insertions(+), 1 deletion(-) diff --git a/drivers/i2c/i2c-core-of-prober.c b/drivers/i2c/i2c-core-of-prober.c index 21f7a4769293..76df75c51cae 100644 --- a/drivers/i2c/i2c-core-of-prober.c +++ b/drivers/i2c/i2c-core-of-prober.c @@ -6,6 +6,7 @@ */ #include +#include #include #include #include @@ -13,6 +14,7 @@ #include #include #include +#include #include #include @@ -29,7 +31,6 @@ * address responds. * * TODO: - * - Support handling common regulators. * - Support handling common GPIOs. * - Support I2C muxes */ @@ -181,3 +182,138 @@ out_put_i2c_adapter: return ret; } EXPORT_SYMBOL_NS_GPL(i2c_of_probe_component, I2C_OF_PROBER); + +static int i2c_of_probe_simple_get_supply(struct device *dev, struct device_node *node, + struct i2c_of_probe_simple_ctx *ctx) +{ + const char *supply_name; + struct regulator *supply; + + /* + * It's entirely possible for the component's device node to not have the + * regulator supplies. While it does not make sense from a hardware perspective, + * the supplies could be always on or otherwise not modeled in the device tree, + * but the device would still work. + */ + supply_name = ctx->opts->supply_name; + if (!supply_name) + return 0; + + supply = of_regulator_get_optional(dev, node, supply_name); + if (IS_ERR(supply)) { + return dev_err_probe(dev, PTR_ERR(supply), + "Failed to get regulator supply \"%s\" from %pOF\n", + supply_name, node); + } + + ctx->supply = supply; + + return 0; +} + +static void i2c_of_probe_simple_put_supply(struct i2c_of_probe_simple_ctx *ctx) +{ + regulator_put(ctx->supply); + ctx->supply = NULL; +} + +static int i2c_of_probe_simple_enable_regulator(struct device *dev, struct i2c_of_probe_simple_ctx *ctx) +{ + int ret; + + if (!ctx->supply) + return 0; + + dev_dbg(dev, "Enabling regulator supply \"%s\"\n", ctx->opts->supply_name); + + ret = regulator_enable(ctx->supply); + if (ret) + return ret; + + if (ctx->opts->post_power_on_delay_ms) + msleep(ctx->opts->post_power_on_delay_ms); + + return 0; +} + +static void i2c_of_probe_simple_disable_regulator(struct device *dev, struct i2c_of_probe_simple_ctx *ctx) +{ + if (!ctx->supply) + return; + + dev_dbg(dev, "Disabling regulator supply \"%s\"\n", ctx->opts->supply_name); + + regulator_disable(ctx->supply); +} + +/** + * i2c_of_probe_simple_enable - Simple helper for I2C OF prober to get and enable resources + * @dev: Pointer to the &struct device of the caller, only used for dev_printk() messages + * @bus_node: Pointer to the &struct device_node of the I2C adapter. + * @data: Pointer to &struct i2c_of_probe_simple_ctx helper context. + * + * If &i2c_of_probe_simple_opts->supply_name is given, request the named regulator supply. + * If a regulator supply was found, enable that regulator. + * + * Return: %0 on success or no-op, or a negative error number on failure. + */ +int i2c_of_probe_simple_enable(struct device *dev, struct device_node *bus_node, void *data) +{ + struct i2c_of_probe_simple_ctx *ctx = data; + struct device_node *node; + const char *compat; + int ret; + + dev_dbg(dev, "Requesting resources for components under I2C bus %pOF\n", bus_node); + + if (!ctx || !ctx->opts) + return -EINVAL; + + compat = ctx->opts->res_node_compatible; + if (!compat) + return -EINVAL; + + node = of_get_compatible_child(bus_node, compat); + if (!node) + return dev_err_probe(dev, -ENODEV, "No device compatible with \"%s\" found\n", + compat); + + ret = i2c_of_probe_simple_get_supply(dev, node, ctx); + if (ret) + goto out_put_node; + + ret = i2c_of_probe_simple_enable_regulator(dev, ctx); + if (ret) + goto out_put_supply; + + return 0; + +out_put_supply: + i2c_of_probe_simple_put_supply(ctx); +out_put_node: + of_node_put(node); + return ret; +} +EXPORT_SYMBOL_NS_GPL(i2c_of_probe_simple_enable, I2C_OF_PROBER); + +/** + * i2c_of_probe_simple_cleanup - Clean up and release resources for I2C OF prober simple helpers + * @dev: Pointer to the &struct device of the caller, only used for dev_printk() messages + * @data: Pointer to &struct i2c_of_probe_simple_ctx helper context. + * + * * If a regulator supply was found, disable that regulator and release it. + */ +void i2c_of_probe_simple_cleanup(struct device *dev, void *data) +{ + struct i2c_of_probe_simple_ctx *ctx = data; + + i2c_of_probe_simple_disable_regulator(dev, ctx); + i2c_of_probe_simple_put_supply(ctx); +} +EXPORT_SYMBOL_NS_GPL(i2c_of_probe_simple_cleanup, I2C_OF_PROBER); + +struct i2c_of_probe_ops i2c_of_probe_simple_ops = { + .enable = i2c_of_probe_simple_enable, + .cleanup = i2c_of_probe_simple_cleanup, +}; +EXPORT_SYMBOL_NS_GPL(i2c_of_probe_simple_ops, I2C_OF_PROBER); diff --git a/include/linux/i2c-of-prober.h b/include/linux/i2c-of-prober.h index e7e052ac9e48..df95aa6ad90e 100644 --- a/include/linux/i2c-of-prober.h +++ b/include/linux/i2c-of-prober.h @@ -70,6 +70,50 @@ struct i2c_of_probe_cfg { int i2c_of_probe_component(struct device *dev, const struct i2c_of_probe_cfg *cfg, void *ctx); +/** + * DOC: I2C OF component prober simple helpers + * + * Components such as trackpads are commonly connected to a devices baseboard + * with a 6-pin ribbon cable. That gives at most one voltage supply and one + * GPIO (commonly a "enable" or "reset" line) besides the I2C bus, interrupt + * pin, and common ground. Touchscreens, while integrated into the display + * panel's connection, typically have the same set of connections. + * + * A simple set of helpers are provided here for use with the I2C OF component + * prober. This implementation targets such components, allowing for at most + * one regulator supply. + * + * The following helpers are provided: + * * i2c_of_probe_simple_enable() + * * i2c_of_probe_simple_cleanup() + */ + +/** + * struct i2c_of_probe_simple_opts - Options for simple I2C component prober callbacks + * @res_node_compatible: Compatible string of device node to retrieve resources from. + * @supply_name: Name of regulator supply. + * @post_power_on_delay_ms: Delay after regulators are powered on. Passed to msleep(). + */ +struct i2c_of_probe_simple_opts { + const char *res_node_compatible; + const char *supply_name; + unsigned int post_power_on_delay_ms; +}; + +struct regulator; + +struct i2c_of_probe_simple_ctx { + /* public: provided by user before helpers are used. */ + const struct i2c_of_probe_simple_opts *opts; + /* private: internal fields for helpers. */ + struct regulator *supply; +}; + +int i2c_of_probe_simple_enable(struct device *dev, struct device_node *bus_node, void *data); +void i2c_of_probe_simple_cleanup(struct device *dev, void *data); + +extern struct i2c_of_probe_ops i2c_of_probe_simple_ops; + #endif /* IS_ENABLED(CONFIG_OF_DYNAMIC) */ #endif /* _LINUX_I2C_OF_PROBER_H */ -- 2.50.1 From 39b415f84654892003cebb7c026b7daa3380610b Mon Sep 17 00:00:00 2001 From: Chen-Yu Tsai Date: Wed, 6 Nov 2024 17:33:32 +0800 Subject: [PATCH 11/16] i2c: of-prober: Add GPIO support to simple helpers Add GPIO support to the simple helpers for the I2C OF component prober. Components that the prober intends to probe likely require their regulator supplies be enabled, and GPIOs be toggled to enable them or bring them out of reset before they will respond to probe attempts. Regulator supplies were handled in the previous patch. The assumption is that the same class of components to be probed are always connected in the same fashion with the same regulator supply and GPIO. The names may vary due to binding differences, but the physical layout does not change. This supports at most one GPIO pin. The user must specify the GPIO name, the polarity, and the amount of time to wait after the GPIO is toggled. Devices with more than one GPIO pin likely require specific power sequencing beyond what generic code can easily support. Signed-off-by: Chen-Yu Tsai Reviewed-by: Douglas Anderson Reviewed-by: Andy Shevchenko Tested-by: Andrey Skvortsov Reviewed-by: AngeloGioacchino Del Regno Signed-off-by: Wolfram Sang --- drivers/i2c/i2c-core-of-prober.c | 100 ++++++++++++++++++++++++++++++- include/linux/i2c-of-prober.h | 21 +++++++ 2 files changed, 119 insertions(+), 2 deletions(-) diff --git a/drivers/i2c/i2c-core-of-prober.c b/drivers/i2c/i2c-core-of-prober.c index 76df75c51cae..b9ca785f8b17 100644 --- a/drivers/i2c/i2c-core-of-prober.c +++ b/drivers/i2c/i2c-core-of-prober.c @@ -10,6 +10,7 @@ #include #include #include +#include #include #include #include @@ -31,7 +32,6 @@ * address responds. * * TODO: - * - Support handling common GPIOs. * - Support I2C muxes */ @@ -246,6 +246,62 @@ static void i2c_of_probe_simple_disable_regulator(struct device *dev, struct i2c regulator_disable(ctx->supply); } +static int i2c_of_probe_simple_get_gpiod(struct device *dev, struct device_node *node, + struct i2c_of_probe_simple_ctx *ctx) +{ + struct fwnode_handle *fwnode = of_fwnode_handle(node); + struct gpio_desc *gpiod; + const char *con_id; + + /* NULL signals no GPIO needed */ + if (!ctx->opts->gpio_name) + return 0; + + /* An empty string signals an unnamed GPIO */ + if (!ctx->opts->gpio_name[0]) + con_id = NULL; + else + con_id = ctx->opts->gpio_name; + + gpiod = fwnode_gpiod_get_index(fwnode, con_id, 0, GPIOD_ASIS, "i2c-of-prober"); + if (IS_ERR(gpiod)) + return PTR_ERR(gpiod); + + ctx->gpiod = gpiod; + + return 0; +} + +static void i2c_of_probe_simple_put_gpiod(struct i2c_of_probe_simple_ctx *ctx) +{ + gpiod_put(ctx->gpiod); + ctx->gpiod = NULL; +} + +static int i2c_of_probe_simple_set_gpio(struct device *dev, struct i2c_of_probe_simple_ctx *ctx) +{ + int ret; + + if (!ctx->gpiod) + return 0; + + dev_dbg(dev, "Configuring GPIO\n"); + + ret = gpiod_direction_output(ctx->gpiod, ctx->opts->gpio_assert_to_enable); + if (ret) + return ret; + + if (ctx->opts->post_gpio_config_delay_ms) + msleep(ctx->opts->post_gpio_config_delay_ms); + + return 0; +} + +static void i2c_of_probe_simple_disable_gpio(struct device *dev, struct i2c_of_probe_simple_ctx *ctx) +{ + gpiod_set_value(ctx->gpiod, !ctx->opts->gpio_assert_to_enable); +} + /** * i2c_of_probe_simple_enable - Simple helper for I2C OF prober to get and enable resources * @dev: Pointer to the &struct device of the caller, only used for dev_printk() messages @@ -253,7 +309,11 @@ static void i2c_of_probe_simple_disable_regulator(struct device *dev, struct i2c * @data: Pointer to &struct i2c_of_probe_simple_ctx helper context. * * If &i2c_of_probe_simple_opts->supply_name is given, request the named regulator supply. + * If &i2c_of_probe_simple_opts->gpio_name is given, request the named GPIO. Or if it is + * the empty string, request the unnamed GPIO. * If a regulator supply was found, enable that regulator. + * If a GPIO line was found, configure the GPIO line to output and set value + * according to given options. * * Return: %0 on success or no-op, or a negative error number on failure. */ @@ -282,12 +342,24 @@ int i2c_of_probe_simple_enable(struct device *dev, struct device_node *bus_node, if (ret) goto out_put_node; - ret = i2c_of_probe_simple_enable_regulator(dev, ctx); + ret = i2c_of_probe_simple_get_gpiod(dev, node, ctx); if (ret) goto out_put_supply; + ret = i2c_of_probe_simple_enable_regulator(dev, ctx); + if (ret) + goto out_put_gpiod; + + ret = i2c_of_probe_simple_set_gpio(dev, ctx); + if (ret) + goto out_disable_regulator; + return 0; +out_disable_regulator: + i2c_of_probe_simple_disable_regulator(dev, ctx); +out_put_gpiod: + i2c_of_probe_simple_put_gpiod(ctx); out_put_supply: i2c_of_probe_simple_put_supply(ctx); out_put_node: @@ -296,17 +368,40 @@ out_put_node: } EXPORT_SYMBOL_NS_GPL(i2c_of_probe_simple_enable, I2C_OF_PROBER); +/** + * i2c_of_probe_simple_cleanup_early - \ + * Simple helper for I2C OF prober to release GPIOs before component is enabled + * @dev: Pointer to the &struct device of the caller; unused. + * @data: Pointer to &struct i2c_of_probe_simple_ctx helper context. + * + * GPIO descriptors are exclusive and have to be released before the + * actual driver probes so that the latter can acquire them. + */ +void i2c_of_probe_simple_cleanup_early(struct device *dev, void *data) +{ + struct i2c_of_probe_simple_ctx *ctx = data; + + i2c_of_probe_simple_put_gpiod(ctx); +} +EXPORT_SYMBOL_NS_GPL(i2c_of_probe_simple_cleanup_early, I2C_OF_PROBER); + /** * i2c_of_probe_simple_cleanup - Clean up and release resources for I2C OF prober simple helpers * @dev: Pointer to the &struct device of the caller, only used for dev_printk() messages * @data: Pointer to &struct i2c_of_probe_simple_ctx helper context. * + * * If a GPIO line was found and not yet released, set its value to the opposite of that + * set in i2c_of_probe_simple_enable() and release it. * * If a regulator supply was found, disable that regulator and release it. */ void i2c_of_probe_simple_cleanup(struct device *dev, void *data) { struct i2c_of_probe_simple_ctx *ctx = data; + /* GPIO operations here are no-ops if i2c_of_probe_simple_cleanup_early was called. */ + i2c_of_probe_simple_disable_gpio(dev, ctx); + i2c_of_probe_simple_put_gpiod(ctx); + i2c_of_probe_simple_disable_regulator(dev, ctx); i2c_of_probe_simple_put_supply(ctx); } @@ -314,6 +409,7 @@ EXPORT_SYMBOL_NS_GPL(i2c_of_probe_simple_cleanup, I2C_OF_PROBER); struct i2c_of_probe_ops i2c_of_probe_simple_ops = { .enable = i2c_of_probe_simple_enable, + .cleanup_early = i2c_of_probe_simple_cleanup_early, .cleanup = i2c_of_probe_simple_cleanup, }; EXPORT_SYMBOL_NS_GPL(i2c_of_probe_simple_ops, I2C_OF_PROBER); diff --git a/include/linux/i2c-of-prober.h b/include/linux/i2c-of-prober.h index df95aa6ad90e..bb6d47f50ee5 100644 --- a/include/linux/i2c-of-prober.h +++ b/include/linux/i2c-of-prober.h @@ -9,6 +9,7 @@ #define _LINUX_I2C_OF_PROBER_H #include +#include struct device; struct device_node; @@ -85,6 +86,7 @@ int i2c_of_probe_component(struct device *dev, const struct i2c_of_probe_cfg *cf * * The following helpers are provided: * * i2c_of_probe_simple_enable() + * * i2c_of_probe_simple_cleanup_early() * * i2c_of_probe_simple_cleanup() */ @@ -92,14 +94,31 @@ int i2c_of_probe_component(struct device *dev, const struct i2c_of_probe_cfg *cf * struct i2c_of_probe_simple_opts - Options for simple I2C component prober callbacks * @res_node_compatible: Compatible string of device node to retrieve resources from. * @supply_name: Name of regulator supply. + * @gpio_name: Name of GPIO. NULL if no GPIO line is used. Empty string ("") if GPIO + * line is unnamed. * @post_power_on_delay_ms: Delay after regulators are powered on. Passed to msleep(). + * @post_gpio_config_delay_ms: Delay after GPIO is configured. Passed to msleep(). + * @gpio_assert_to_enable: %true if GPIO should be asserted, i.e. set to logical high, + * to enable the component. + * + * This describes power sequences common for the class of components supported by the + * simple component prober: + * * @gpio_name is configured to the non-active setting according to @gpio_assert_to_enable. + * * @supply_name regulator supply is enabled. + * * Wait for @post_power_on_delay_ms to pass. + * * @gpio_name is configured to the active setting according to @gpio_assert_to_enable. + * * Wait for @post_gpio_config_delay_ms to pass. */ struct i2c_of_probe_simple_opts { const char *res_node_compatible; const char *supply_name; + const char *gpio_name; unsigned int post_power_on_delay_ms; + unsigned int post_gpio_config_delay_ms; + bool gpio_assert_to_enable; }; +struct gpio_desc; struct regulator; struct i2c_of_probe_simple_ctx { @@ -107,9 +126,11 @@ struct i2c_of_probe_simple_ctx { const struct i2c_of_probe_simple_opts *opts; /* private: internal fields for helpers. */ struct regulator *supply; + struct gpio_desc *gpiod; }; int i2c_of_probe_simple_enable(struct device *dev, struct device_node *bus_node, void *data); +void i2c_of_probe_simple_cleanup_early(struct device *dev, void *data); void i2c_of_probe_simple_cleanup(struct device *dev, void *data); extern struct i2c_of_probe_ops i2c_of_probe_simple_ops; -- 2.50.1 From 3fc361af8ab0a96619ba0146a5f694f59ae3f4c2 Mon Sep 17 00:00:00 2001 From: Chen-Yu Tsai Date: Wed, 6 Nov 2024 17:33:33 +0800 Subject: [PATCH 12/16] platform/chrome: Introduce device tree hardware prober Some devices are designed and manufactured with some components having multiple drop-in replacement options. These components are often connected to the mainboard via ribbon cables, having the same signals and pin assignments across all options. These may include the display panel and touchscreen on laptops and tablets, and the trackpad on laptops. Sometimes which component option is used in a particular device can be detected by some firmware provided identifier, other times that information is not available, and the kernel has to try to probe each device. This change attempts to make the "probe each device" case cleaner. The current approach is to have all options added and enabled in the device tree. The kernel would then bind each device and run each driver's probe function. This works, but has been broken before due to the introduction of asynchronous probing, causing multiple instances requesting "shared" resources, such as pinmuxes, GPIO pins, interrupt lines, at the same time, with only one instance succeeding. Work arounds for these include moving the pinmux to the parent I2C controller, using GPIO hogs or pinmux settings to keep the GPIO pins in some fixed configuration, and requesting the interrupt line very late. Such configurations can be seen on the MT8183 Krane Chromebook tablets, and the Qualcomm sc8280xp-based Lenovo Thinkpad 13S. Instead of this delicate dance between drivers and device tree quirks, this change introduces a simple I2C component prober. For any given class of devices on the same I2C bus, it will go through all of them, doing a simple I2C read transfer and see which one of them responds. It will then enable the device that responds. This requires some minor modifications in the existing device tree. The status for all the device nodes for the component options must be set to "fail-needs-probe". This makes it clear that some mechanism is needed to enable one of them, and also prevents the prober and device drivers running at the same time. Signed-off-by: Chen-Yu Tsai Acked-by: Tzung-Bi Shih Reviewed-by: Douglas Anderson Reviewed-by: AngeloGioacchino Del Regno Signed-off-by: Wolfram Sang --- drivers/platform/chrome/Kconfig | 11 ++ drivers/platform/chrome/Makefile | 1 + .../platform/chrome/chromeos_of_hw_prober.c | 154 ++++++++++++++++++ 3 files changed, 166 insertions(+) create mode 100644 drivers/platform/chrome/chromeos_of_hw_prober.c diff --git a/drivers/platform/chrome/Kconfig b/drivers/platform/chrome/Kconfig index 7dbeb786352a..b7dbaf77b6db 100644 --- a/drivers/platform/chrome/Kconfig +++ b/drivers/platform/chrome/Kconfig @@ -61,6 +61,17 @@ config CHROMEOS_TBMC To compile this driver as a module, choose M here: the module will be called chromeos_tbmc. +config CHROMEOS_OF_HW_PROBER + tristate "ChromeOS Device Tree Hardware Prober" + depends on OF + depends on I2C + select OF_DYNAMIC + default OF + help + This option enables the device tree hardware prober for ChromeOS + devices. The driver will probe the correct component variant in + devices that have multiple drop-in options for one component. + config CROS_EC tristate "ChromeOS Embedded Controller" select CROS_EC_PROTO diff --git a/drivers/platform/chrome/Makefile b/drivers/platform/chrome/Makefile index 2dcc6ccc2302..fb8335458a22 100644 --- a/drivers/platform/chrome/Makefile +++ b/drivers/platform/chrome/Makefile @@ -6,6 +6,7 @@ CFLAGS_cros_ec_sensorhub_ring.o:= -I$(src) obj-$(CONFIG_CHROMEOS_ACPI) += chromeos_acpi.o obj-$(CONFIG_CHROMEOS_LAPTOP) += chromeos_laptop.o +obj-$(CONFIG_CHROMEOS_OF_HW_PROBER) += chromeos_of_hw_prober.o obj-$(CONFIG_CHROMEOS_PRIVACY_SCREEN) += chromeos_privacy_screen.o obj-$(CONFIG_CHROMEOS_PSTORE) += chromeos_pstore.o obj-$(CONFIG_CHROMEOS_TBMC) += chromeos_tbmc.o diff --git a/drivers/platform/chrome/chromeos_of_hw_prober.c b/drivers/platform/chrome/chromeos_of_hw_prober.c new file mode 100644 index 000000000000..297d4704b75f --- /dev/null +++ b/drivers/platform/chrome/chromeos_of_hw_prober.c @@ -0,0 +1,154 @@ +// SPDX-License-Identifier: GPL-2.0-only +/* + * ChromeOS Device Tree Hardware Prober + * + * Copyright (c) 2024 Google LLC + */ + +#include +#include +#include +#include +#include +#include +#include + +#define DRV_NAME "chromeos_of_hw_prober" + +/** + * struct hw_prober_entry - Holds an entry for the hardware prober + * + * @compatible: compatible string to match against the machine + * @prober: prober function to call when machine matches + * @data: extra data for the prober function + */ +struct hw_prober_entry { + const char *compatible; + int (*prober)(struct device *dev, const void *data); + const void *data; +}; + +struct chromeos_i2c_probe_data { + const struct i2c_of_probe_cfg *cfg; + const struct i2c_of_probe_simple_opts *opts; +}; + +static int chromeos_i2c_component_prober(struct device *dev, const void *_data) +{ + const struct chromeos_i2c_probe_data *data = _data; + struct i2c_of_probe_simple_ctx ctx = { + .opts = data->opts, + }; + + return i2c_of_probe_component(dev, data->cfg, &ctx); +} + +#define DEFINE_CHROMEOS_I2C_PROBE_CFG_SIMPLE_BY_TYPE(_type) \ + static const struct i2c_of_probe_cfg chromeos_i2c_probe_simple_ ## _type ## _cfg = { \ + .type = #_type, \ + .ops = &i2c_of_probe_simple_ops, \ + } + +#define DEFINE_CHROMEOS_I2C_PROBE_DATA_DUMB_BY_TYPE(_type) \ + static const struct chromeos_i2c_probe_data chromeos_i2c_probe_dumb_ ## _type = { \ + .cfg = &(const struct i2c_of_probe_cfg) { \ + .type = #_type, \ + }, \ + } + +DEFINE_CHROMEOS_I2C_PROBE_DATA_DUMB_BY_TYPE(touchscreen); + +DEFINE_CHROMEOS_I2C_PROBE_CFG_SIMPLE_BY_TYPE(trackpad); + +static const struct chromeos_i2c_probe_data chromeos_i2c_probe_hana_trackpad = { + .cfg = &chromeos_i2c_probe_simple_trackpad_cfg, + .opts = &(const struct i2c_of_probe_simple_opts) { + .res_node_compatible = "elan,ekth3000", + .supply_name = "vcc", + /* + * ELAN trackpad needs 2 ms for H/W init and 100 ms for F/W init. + * Synaptics trackpad needs 100 ms. + * However, the regulator is set to "always-on", presumably to + * avoid this delay. The ELAN driver is also missing delays. + */ + .post_power_on_delay_ms = 0, + }, +}; + +static const struct hw_prober_entry hw_prober_platforms[] = { + { + .compatible = "google,hana", + .prober = chromeos_i2c_component_prober, + .data = &chromeos_i2c_probe_dumb_touchscreen, + }, { + .compatible = "google,hana", + .prober = chromeos_i2c_component_prober, + .data = &chromeos_i2c_probe_hana_trackpad, + }, +}; + +static int chromeos_of_hw_prober_probe(struct platform_device *pdev) +{ + for (size_t i = 0; i < ARRAY_SIZE(hw_prober_platforms); i++) { + int ret; + + if (!of_machine_is_compatible(hw_prober_platforms[i].compatible)) + continue; + + ret = hw_prober_platforms[i].prober(&pdev->dev, hw_prober_platforms[i].data); + /* Ignore unrecoverable errors and keep going through other probers */ + if (ret == -EPROBE_DEFER) + return ret; + } + + return 0; +} + +static struct platform_driver chromeos_of_hw_prober_driver = { + .probe = chromeos_of_hw_prober_probe, + .driver = { + .name = DRV_NAME, + }, +}; + +static struct platform_device *chromeos_of_hw_prober_pdev; + +static int chromeos_of_hw_prober_driver_init(void) +{ + size_t i; + int ret; + + for (i = 0; i < ARRAY_SIZE(hw_prober_platforms); i++) + if (of_machine_is_compatible(hw_prober_platforms[i].compatible)) + break; + if (i == ARRAY_SIZE(hw_prober_platforms)) + return -ENODEV; + + ret = platform_driver_register(&chromeos_of_hw_prober_driver); + if (ret) + return ret; + + chromeos_of_hw_prober_pdev = + platform_device_register_simple(DRV_NAME, PLATFORM_DEVID_NONE, NULL, 0); + if (IS_ERR(chromeos_of_hw_prober_pdev)) + goto err; + + return 0; + +err: + platform_driver_unregister(&chromeos_of_hw_prober_driver); + + return PTR_ERR(chromeos_of_hw_prober_pdev); +} +module_init(chromeos_of_hw_prober_driver_init); + +static void chromeos_of_hw_prober_driver_exit(void) +{ + platform_device_unregister(chromeos_of_hw_prober_pdev); + platform_driver_unregister(&chromeos_of_hw_prober_driver); +} +module_exit(chromeos_of_hw_prober_driver_exit); + +MODULE_LICENSE("GPL"); +MODULE_DESCRIPTION("ChromeOS device tree hardware prober"); +MODULE_IMPORT_NS(I2C_OF_PROBER); -- 2.50.1 From aac9e2afa807e862073e9536099a3184b0e936d2 Mon Sep 17 00:00:00 2001 From: Chen-Yu Tsai Date: Wed, 6 Nov 2024 17:33:34 +0800 Subject: [PATCH 13/16] arm64: dts: mediatek: mt8173-elm-hana: Mark touchscreens and trackpads as fail Instead of having them all available, mark them all as "fail-needs-probe" and have the implementation try to probe which one is present. Also remove the shared resource workaround by moving the pinctrl entry for the trackpad interrupt line back into the individual trackpad nodes. Cc: # Needs accompanying new driver to work Signed-off-by: Chen-Yu Tsai Reviewed-by: Douglas Anderson Reviewed-by: AngeloGioacchino Del Regno Acked-by: AngeloGioacchino Del Regno Signed-off-by: Wolfram Sang --- arch/arm64/boot/dts/mediatek/mt8173-elm-hana.dtsi | 14 ++++++++++++++ arch/arm64/boot/dts/mediatek/mt8173-elm.dtsi | 4 ++-- 2 files changed, 16 insertions(+), 2 deletions(-) diff --git a/arch/arm64/boot/dts/mediatek/mt8173-elm-hana.dtsi b/arch/arm64/boot/dts/mediatek/mt8173-elm-hana.dtsi index ae0379fd42a9..dfc5c2f0ddef 100644 --- a/arch/arm64/boot/dts/mediatek/mt8173-elm-hana.dtsi +++ b/arch/arm64/boot/dts/mediatek/mt8173-elm-hana.dtsi @@ -14,6 +14,7 @@ compatible = "melfas,mip4_ts"; reg = <0x34>; interrupts-extended = <&pio 88 IRQ_TYPE_LEVEL_LOW>; + status = "fail-needs-probe"; }; /* @@ -26,6 +27,7 @@ reg = <0x20>; hid-descr-addr = <0x0020>; interrupts-extended = <&pio 88 IRQ_TYPE_LEVEL_LOW>; + status = "fail-needs-probe"; }; /* Lenovo Ideapad C330 uses G2Touch touchscreen as a 2nd source touchscreen */ @@ -35,6 +37,7 @@ hid-descr-addr = <0x0001>; interrupt-parent = <&pio>; interrupts = <88 IRQ_TYPE_LEVEL_LOW>; + status = "fail-needs-probe"; }; }; @@ -47,6 +50,8 @@ trackpad2: trackpad@2c { compatible = "hid-over-i2c"; interrupts-extended = <&pio 117 IRQ_TYPE_LEVEL_LOW>; + pinctrl-names = "default"; + pinctrl-0 = <&trackpad_irq>; reg = <0x2c>; hid-descr-addr = <0x0020>; /* @@ -58,6 +63,7 @@ */ vdd-supply = <&mt6397_vgp6_reg>; wakeup-source; + status = "fail-needs-probe"; }; }; @@ -82,3 +88,11 @@ }; }; }; + +&touchscreen { + status = "fail-needs-probe"; +}; + +&trackpad { + status = "fail-needs-probe"; +}; diff --git a/arch/arm64/boot/dts/mediatek/mt8173-elm.dtsi b/arch/arm64/boot/dts/mediatek/mt8173-elm.dtsi index b4d85147b77b..eee64461421f 100644 --- a/arch/arm64/boot/dts/mediatek/mt8173-elm.dtsi +++ b/arch/arm64/boot/dts/mediatek/mt8173-elm.dtsi @@ -358,12 +358,12 @@ &i2c4 { clock-frequency = <400000>; status = "okay"; - pinctrl-names = "default"; - pinctrl-0 = <&trackpad_irq>; trackpad: trackpad@15 { compatible = "elan,ekth3000"; interrupts-extended = <&pio 117 IRQ_TYPE_LEVEL_LOW>; + pinctrl-names = "default"; + pinctrl-0 = <&trackpad_irq>; reg = <0x15>; vcc-supply = <&mt6397_vgp6_reg>; wakeup-source; -- 2.50.1 From 44b682694a0ca366bf15c26c3c3c16d26c9e9f6d Mon Sep 17 00:00:00 2001 From: Liam Zuiderhoek Date: Tue, 22 Oct 2024 20:46:59 +0200 Subject: [PATCH 14/16] i2c: Fix whitespace style issue This patch fixes a coding style issue in the alignment of parameters in the function i2c_smbus_write_bytes(). It replaces spaces with tabs for alignment, as per the coding style guidelines. Signed-off-by: Liam Zuiderhoek Signed-off-by: Wolfram Sang --- drivers/i2c/i2c-core-smbus.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/i2c/i2c-core-smbus.c b/drivers/i2c/i2c-core-smbus.c index 0a6bdc6df172..e73afbefe222 100644 --- a/drivers/i2c/i2c-core-smbus.c +++ b/drivers/i2c/i2c-core-smbus.c @@ -122,7 +122,7 @@ EXPORT_SYMBOL(i2c_smbus_read_byte); s32 i2c_smbus_write_byte(const struct i2c_client *client, u8 value) { return i2c_smbus_xfer(client->adapter, client->addr, client->flags, - I2C_SMBUS_WRITE, value, I2C_SMBUS_BYTE, NULL); + I2C_SMBUS_WRITE, value, I2C_SMBUS_BYTE, NULL); } EXPORT_SYMBOL(i2c_smbus_write_byte); -- 2.50.1 From 0d40daa1c1369c2fef6a40f44640b2e5f3453daa Mon Sep 17 00:00:00 2001 From: Chen-Yu Tsai Date: Thu, 28 Nov 2024 12:26:30 +0800 Subject: [PATCH 15/16] of: base: Document prefix argument for of_get_next_child_with_prefix() When of_get_next_child_with_prefix() was added, the prefix argument was left undocumented. This caused a new warning to be generated during the kerneldoc build process: drivers/of/base.c:661: warning: Function parameter or struct member 'prefix' not described in 'of_get_next_child_with_prefix' Properly document the argument to fix this. Reported-by: kernel test robot Closes: https://lore.kernel.org/oe-kbuild-all/202411280010.KGSDBOUE-lkp@intel.com/ Fixes: 1fcc67e3a354 ("of: base: Add for_each_child_of_node_with_prefix()") Signed-off-by: Chen-Yu Tsai Signed-off-by: Wolfram Sang --- drivers/of/base.c | 1 + 1 file changed, 1 insertion(+) diff --git a/drivers/of/base.c b/drivers/of/base.c index 4cba021c89d3..7dc394255a0a 100644 --- a/drivers/of/base.c +++ b/drivers/of/base.c @@ -648,6 +648,7 @@ EXPORT_SYMBOL(of_get_next_child); * of_get_next_child_with_prefix - Find the next child node with prefix * @node: parent node * @prev: previous child of the parent node, or NULL to get first + * @prefix: prefix that the node name should have * * This function is like of_get_next_child(), except that it automatically * skips any nodes whose name doesn't have the given prefix. -- 2.50.1 From caf4bdb558cbc9893524b0a15e6423ee6305cb0c Mon Sep 17 00:00:00 2001 From: Lukas Bulwahn Date: Fri, 29 Nov 2024 10:52:38 +0100 Subject: [PATCH 16/16] MAINTAINERS: fix typo in I2C OF COMPONENT PROBER Commit 157ce8f381ef ("i2c: Introduce OF component probe function") adds the header file include/linux/i2c-of-prober.h and a corresponding file entry in the newly added MAINTAINERS section I2C OF COMPONENT PROBER. This file entry unfortunately has a typo. Fortunately, ./scripts/get_maintainer.pl --self-test=patterns detects this broken reference. Fix the typo in this file entry in the I2C OF COMPONENT PROBER section. Fixes: 157ce8f381ef ("i2c: Introduce OF component probe function") Signed-off-by: Lukas Bulwahn Signed-off-by: Wolfram Sang --- MAINTAINERS | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/MAINTAINERS b/MAINTAINERS index 0f837aca327c..e4a840545dcb 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -10708,7 +10708,7 @@ L: linux-i2c@vger.kernel.org L: devicetree@vger.kernel.org S: Maintained F: drivers/i2c/i2c-core-of-prober.c -F: include/linux-i2c-of-prober.h +F: include/linux/i2c-of-prober.h I2C OVER PARALLEL PORT M: Jean Delvare -- 2.50.1