| Message ID | 20240626-qps615-v1-7-2ade7bd91e02@quicinc.com (mailing list archive) |
|---|---|
| State | RFC |
| Delegated to: | Krzysztof Wilczyński |
| Headers | show |
| Series | PCI: enable Power and configure the QPS615 PCIe switch | expand |
On Wed, Jun 26, 2024 at 2:38 PM Krishna chaitanya chundru <quic_krichai@quicinc.com> wrote: > > QPS615 switch needs to configured after powering on and before > PCIe link was up. > > As the PCIe controller driver already enables the PCIe link training > at the host side, stop the link training. > Otherwise the moment we turn on the switch it will participate in > the link training and link may come before switch is configured through > i2c. > > The switch can be configured different ways like changing de-emphasis > settings of the switch, disabling unused ports etc and these settings > can vary from board to board, for that reason the sequence is taken > from the firmware file which contains the address of the slave, to address > and data to be written to the switch. The driver reads the firmware file > and parses them to apply those configurations to the switch. > > Signed-off-by: Krishna chaitanya chundru <quic_krichai@quicinc.com> > --- > drivers/pci/pwrctl/Kconfig | 7 + > drivers/pci/pwrctl/Makefile | 1 + > drivers/pci/pwrctl/pci-pwrctl-qps615.c | 278 +++++++++++++++++++++++++++++++++ > 3 files changed, 286 insertions(+) > > diff --git a/drivers/pci/pwrctl/Kconfig b/drivers/pci/pwrctl/Kconfig > index f1b824955d4b..a419b250006d 100644 > --- a/drivers/pci/pwrctl/Kconfig > +++ b/drivers/pci/pwrctl/Kconfig > @@ -14,4 +14,11 @@ config PCI_PWRCTL_PWRSEQ > Enable support for the PCI power control driver for device > drivers using the Power Sequencing subsystem. > > +config PCI_PWRCTL_QPS615 > + tristate "PCI Power Control driver for QPS615" > + select PCI_PWRCTL > + help > + Enable support for the PCI power control driver for QPS615 and > + configures it. > + > endmenu > diff --git a/drivers/pci/pwrctl/Makefile b/drivers/pci/pwrctl/Makefile > index d308aae4800c..ac563a70c023 100644 > --- a/drivers/pci/pwrctl/Makefile > +++ b/drivers/pci/pwrctl/Makefile > @@ -4,3 +4,4 @@ obj-$(CONFIG_PCI_PWRCTL) += pci-pwrctl-core.o > pci-pwrctl-core-y := core.o > > obj-$(CONFIG_PCI_PWRCTL_PWRSEQ) += pci-pwrctl-pwrseq.o > +obj-$(CONFIG_PCI_PWRCTL_QPS615) += pci-pwrctl-qps615.o > diff --git a/drivers/pci/pwrctl/pci-pwrctl-qps615.c b/drivers/pci/pwrctl/pci-pwrctl-qps615.c > new file mode 100644 > index 000000000000..1f2caf5d7da2 > --- /dev/null > +++ b/drivers/pci/pwrctl/pci-pwrctl-qps615.c > @@ -0,0 +1,278 @@ > +// SPDX-License-Identifier: GPL-2.0-only > +/* > + * Copyright (c) 2023 Qualcomm Innovation Center, Inc. All rights reserved. 2024? > + */ > + > +#include <linux/device.h> > +#include <linux/firmware.h> > +#include <linux/i2c.h> > +#include <linux/mod_devicetable.h> > +#include <linux/module.h> > +#include <linux/of.h> > +#include <linux/pci-pwrctl.h> > +#include <linux/platform_device.h> > +#include <linux/regulator/consumer.h> > +#include <linux/string.h> > +#include <linux/types.h> > + > +#include "../pci.h" > + > +struct qcom_qps615_pwrctl_i2c_setting { > + u32 slv_addr; > + u32 reg_addr; > + u32 val; > +}; > + > +struct qcom_qps615_pwrctl_ctx { > + struct i2c_adapter *adapter; > + struct pci_pwrctl pwrctl; > + struct device_link *link; > + struct regulator *vdd; > +}; > + > +/* write 32-bit value to 24 bit register */ > +static int qps615_switch_i2c_write(struct qcom_qps615_pwrctl_ctx *ctx, u32 slv_addr, u32 reg_addr, Break the line after "*ctx" unless you have a good reason for long lines? > + u32 reg_val) > +{ > + struct i2c_msg msg; > + u8 msg_buf[7]; > + int ret; > + > + msg.addr = slv_addr; > + msg.len = 7; > + msg.flags = 0; > + > + /* Big Endian for reg addr */ > + msg_buf[0] = (u8)(reg_addr >> 16); > + msg_buf[1] = (u8)(reg_addr >> 8); > + msg_buf[2] = (u8)reg_addr; > + > + /* Little Endian for reg val */ > + msg_buf[3] = (u8)(reg_val); > + msg_buf[4] = (u8)(reg_val >> 8); > + msg_buf[5] = (u8)(reg_val >> 16); > + msg_buf[6] = (u8)(reg_val >> 24); > + > + msg.buf = msg_buf; > + ret = i2c_transfer(ctx->adapter, &msg, 1); > + return ret == 1 ? 0 : ret; > +} > + > +/* read 32 bit value from 24 bit reg addr */ > +static int qps615_switch_i2c_read(struct qcom_qps615_pwrctl_ctx *ctx, u32 slv_addr, u32 reg_addr, > + u32 *reg_val) > +{ > + u8 wr_data[3], rd_data[4] = {0}; > + struct i2c_msg msg[2]; > + int ret; > + > + msg[0].addr = slv_addr; > + msg[0].len = 3; > + msg[0].flags = 0; > + > + /* Big Endian for reg addr */ > + wr_data[0] = (u8)(reg_addr >> 16); > + wr_data[1] = (u8)(reg_addr >> 8); > + wr_data[2] = (u8)reg_addr; > + > + msg[0].buf = wr_data; > + > + msg[1].addr = slv_addr; > + msg[1].len = 4; > + msg[1].flags = I2C_M_RD; > + > + msg[1].buf = rd_data; > + > + ret = i2c_transfer(ctx->adapter, &msg[0], 2); > + if (ret != 2) > + return ret; > + > + *reg_val = (rd_data[3] << 24) | (rd_data[2] << 16) | (rd_data[1] << 8) | rd_data[0]; > + > + return 0; > +} > + > +static int qcom_qps615_pwrctl_init(struct qcom_qps615_pwrctl_ctx *ctx) > +{ > + struct device *dev = ctx->pwrctl.dev; > + struct qcom_qps615_pwrctl_i2c_setting *set; > + const struct firmware *fw; > + const u8 *pos, *eof; > + int ret; > + u32 val; > + > + ret = request_firmware(&fw, "qcom/qps615.bin", dev); > + if (ret < 0) { > + dev_err(dev, "firmware loading failed with ret %d\n", ret); > + return ret; > + } > + > + if (!fw) { > + ret = -EINVAL; > + goto err; > + } > + > + pos = fw->data; > + eof = fw->data + fw->size; > + > + while (pos < (fw->data + fw->size)) { > + set = (struct qcom_qps615_pwrctl_i2c_setting *)pos; > + > + ret = qps615_switch_i2c_write(ctx, set->slv_addr, set->reg_addr, set->val); > + if (ret) { > + dev_err(dev, > + "I2c write failed for slv addr:%x at addr%x with val %x ret %d\n", > + set->slv_addr, set->reg_addr, set->val, ret); > + goto err; > + } > + > + ret = qps615_switch_i2c_read(ctx, set->slv_addr, set->reg_addr, &val); > + if (ret) { > + dev_err(dev, "I2c read failed for slv addr:%x at addr%x ret %d\n", > + set->slv_addr, set->reg_addr, ret); > + goto err; > + } > + > + if (set->val != val) { > + dev_err(dev, > + "I2c read's mismatch for slv:%x at addr%x exp%d got%d\n", > + set->slv_addr, set->reg_addr, set->val, val); > + goto err; > + } > + pos += sizeof(struct qcom_qps615_pwrctl_i2c_setting); > + } > + > +err: > + release_firmware(fw); > + > + return ret; > +} > + > +static int qcom_qps615_power_on(struct qcom_qps615_pwrctl_ctx *ctx) > +{ > + int ret; > + > + ret = regulator_enable(ctx->vdd); > + if (ret) { > + dev_err(ctx->pwrctl.dev, "cannot enable vdda regulator\n"); Use dev_err_probe() here and wherever applicable. > + return ret; > + } > + > + ret = qcom_qps615_pwrctl_init(ctx); > + if (ret) > + regulator_disable(ctx->vdd); > + > + return ret; > +} > + > +static int qcom_qps615_power_off(struct qcom_qps615_pwrctl_ctx *ctx) > +{ > + return regulator_disable(ctx->vdd); > +} > + > +static int qcom_qps615_pwrctl_probe(struct platform_device *pdev) > +{ > + struct device *dev = &pdev->dev; > + struct device_node *node = pdev->dev.of_node; > + struct qcom_qps615_pwrctl_ctx *ctx; > + struct device_node *adapter_node; > + struct pci_host_bridge *bridge; > + struct i2c_adapter *adapter; > + struct pci_bus *bus; > + > + bus = pci_find_bus(of_get_pci_domain_nr(dev->parent->of_node), 0); > + if (!bus) > + return -ENODEV; > + > + bridge = pci_find_host_bridge(bus); > + > + ctx = devm_kzalloc(dev, sizeof(*ctx), GFP_KERNEL); > + if (!ctx) > + return -ENOMEM; > + > + adapter_node = of_parse_phandle(node, "switch-i2c-cntrl", 0); > + if (adapter_node) { > + adapter = of_get_i2c_adapter_by_node(adapter_node); > + __free(adapter_node); How did this even compile??! I think you meant: struct device_node *adapter_node __free(device_node) = ... ? Bart > + if (!adapter) > + return dev_err_probe(dev, -EPROBE_DEFER, > + "failed to parse switch-i2c-cntrl\n"); > + } > + > + ctx->pwrctl.dev = dev; > + ctx->adapter = adapter; > + ctx->vdd = devm_regulator_get(dev, "vdd"); > + if (IS_ERR(ctx->vdd)) > + return dev_err_probe(dev, PTR_ERR(ctx->vdd), > + "failed to get vdd regulator\n"); > + > + ctx->link = device_link_add(&bridge->dev, dev, DL_FLAG_AUTOREMOVE_CONSUMER); > + > + platform_set_drvdata(pdev, ctx); > + > + bridge->ops->stop_link(bus); > + qcom_qps615_power_on(ctx); > + bridge->ops->start_link(bus); > + > + return devm_pci_pwrctl_device_set_ready(dev, &ctx->pwrctl); > +} > + > +static const struct of_device_id qcom_qps615_pwrctl_of_match[] = { > + { > + .compatible = "pci1179,0623", > + }, > + { } > +}; > +MODULE_DEVICE_TABLE(of, qcom_qps615_pwrctl_of_match); > + > +static int pci_pwrctl_suspend_noirq(struct device *dev) > +{ > + struct pci_bus *bus = pci_find_bus(of_get_pci_domain_nr(dev->parent->of_node), 0); > + struct pci_host_bridge *bridge = pci_find_host_bridge(bus); > + struct qcom_qps615_pwrctl_ctx *ctx = dev_get_drvdata(dev); > + > + bridge->ops->stop_link(bus); > + qcom_qps615_power_off(ctx); > + > + return 0; > +} > + > +static int pci_pwrctl_resume_noirq(struct device *dev) > +{ > + struct pci_bus *bus = pci_find_bus(of_get_pci_domain_nr(dev->parent->of_node), 0); > + struct pci_host_bridge *bridge = pci_find_host_bridge(bus); > + struct qcom_qps615_pwrctl_ctx *ctx = dev_get_drvdata(dev); > + > + qcom_qps615_power_on(ctx); > + bridge->ops->start_link(bus); > + > + return 0; > +} > + > +static void qcom_qps615_pwrctl_remove(struct platform_device *pdev) > +{ > + struct device *dev = &pdev->dev; > + struct qcom_qps615_pwrctl_ctx *ctx = dev_get_drvdata(dev); > + > + device_link_del(ctx->link); > + pci_pwrctl_suspend_noirq(dev); > +} > + > +static const struct dev_pm_ops pci_pwrctl_pm_ops = { > + NOIRQ_SYSTEM_SLEEP_PM_OPS(pci_pwrctl_suspend_noirq, pci_pwrctl_resume_noirq) > +}; > + > +static struct platform_driver qcom_qps615_pwrctl_driver = { > + .driver = { > + .name = "pwrctl-qps615", > + .of_match_table = qcom_qps615_pwrctl_of_match, > + .pm = &pci_pwrctl_pm_ops, > + }, > + .probe = qcom_qps615_pwrctl_probe, > + .remove_new = qcom_qps615_pwrctl_remove, > +}; > +module_platform_driver(qcom_qps615_pwrctl_driver); > + > +MODULE_AUTHOR("Krishna chaitanya chundru <quic_krichai@quicinc.com>"); > +MODULE_DESCRIPTION("Qualcomm QPS615 power control driver"); > +MODULE_LICENSE("GPL"); > > -- > 2.42.0 >
On 26.06.2024 2:37 PM, Krishna chaitanya chundru wrote: > QPS615 switch needs to configured after powering on and before > PCIe link was up. > > As the PCIe controller driver already enables the PCIe link training > at the host side, stop the link training. > Otherwise the moment we turn on the switch it will participate in > the link training and link may come before switch is configured through > i2c. > > The switch can be configured different ways like changing de-emphasis > settings of the switch, disabling unused ports etc and these settings > can vary from board to board, for that reason the sequence is taken > from the firmware file which contains the address of the slave, to address > and data to be written to the switch. The driver reads the firmware file > and parses them to apply those configurations to the switch. > > Signed-off-by: Krishna chaitanya chundru <quic_krichai@quicinc.com> > --- [...] > +static int qcom_qps615_pwrctl_init(struct qcom_qps615_pwrctl_ctx *ctx) > +{ > + struct device *dev = ctx->pwrctl.dev; > + struct qcom_qps615_pwrctl_i2c_setting *set; > + const struct firmware *fw; > + const u8 *pos, *eof; > + int ret; > + u32 val; > + > + ret = request_firmware(&fw, "qcom/qps615.bin", dev); Is this driver only going to serve one model of the device, that will use this specific firmware file, ever? In other words, is QPS615 super special and no other chip like it will be ever made? [...] > + > + bridge->ops->stop_link(bus); This is turbo intrusive. What if there are more devices on this bus? Konrad
On Wed, Jun 26, 2024 at 06:07:55PM GMT, Krishna chaitanya chundru wrote: > QPS615 switch needs to configured after powering on and before > PCIe link was up. "before PCIe link is brought up"? > > As the PCIe controller driver already enables the PCIe link training > at the host side, stop the link training. > Otherwise the moment we turn on the switch it will participate in > the link training and link may come before switch is configured through > i2c. > > The switch can be configured different ways like changing de-emphasis > settings of the switch, disabling unused ports etc and these settings > can vary from board to board, for that reason the sequence is taken > from the firmware file which contains the address of the slave, to address > and data to be written to the switch. The driver reads the firmware file > and parses them to apply those configurations to the switch. > So the firmware file contains calibration data only? How big is this, should/could it be provided in driver with calibration settings from DeviceTree? > Signed-off-by: Krishna chaitanya chundru <quic_krichai@quicinc.com> > --- > drivers/pci/pwrctl/Kconfig | 7 + > drivers/pci/pwrctl/Makefile | 1 + > drivers/pci/pwrctl/pci-pwrctl-qps615.c | 278 +++++++++++++++++++++++++++++++++ > 3 files changed, 286 insertions(+) > > diff --git a/drivers/pci/pwrctl/Kconfig b/drivers/pci/pwrctl/Kconfig > index f1b824955d4b..a419b250006d 100644 > --- a/drivers/pci/pwrctl/Kconfig > +++ b/drivers/pci/pwrctl/Kconfig > @@ -14,4 +14,11 @@ config PCI_PWRCTL_PWRSEQ > Enable support for the PCI power control driver for device > drivers using the Power Sequencing subsystem. > > +config PCI_PWRCTL_QPS615 > + tristate "PCI Power Control driver for QPS615" > + select PCI_PWRCTL > + help > + Enable support for the PCI power control driver for QPS615 and > + configures it. > + > endmenu > diff --git a/drivers/pci/pwrctl/Makefile b/drivers/pci/pwrctl/Makefile > index d308aae4800c..ac563a70c023 100644 > --- a/drivers/pci/pwrctl/Makefile > +++ b/drivers/pci/pwrctl/Makefile > @@ -4,3 +4,4 @@ obj-$(CONFIG_PCI_PWRCTL) += pci-pwrctl-core.o > pci-pwrctl-core-y := core.o > > obj-$(CONFIG_PCI_PWRCTL_PWRSEQ) += pci-pwrctl-pwrseq.o > +obj-$(CONFIG_PCI_PWRCTL_QPS615) += pci-pwrctl-qps615.o > diff --git a/drivers/pci/pwrctl/pci-pwrctl-qps615.c b/drivers/pci/pwrctl/pci-pwrctl-qps615.c > new file mode 100644 > index 000000000000..1f2caf5d7da2 > --- /dev/null > +++ b/drivers/pci/pwrctl/pci-pwrctl-qps615.c > @@ -0,0 +1,278 @@ > +// SPDX-License-Identifier: GPL-2.0-only > +/* > + * Copyright (c) 2023 Qualcomm Innovation Center, Inc. All rights reserved. > + */ > + > +#include <linux/device.h> > +#include <linux/firmware.h> > +#include <linux/i2c.h> > +#include <linux/mod_devicetable.h> > +#include <linux/module.h> > +#include <linux/of.h> > +#include <linux/pci-pwrctl.h> > +#include <linux/platform_device.h> > +#include <linux/regulator/consumer.h> > +#include <linux/string.h> > +#include <linux/types.h> > + > +#include "../pci.h" > + > +struct qcom_qps615_pwrctl_i2c_setting { > + u32 slv_addr; Prefer this to be named e.g. device_addr Or perhaps use "addr", "offset", and "value"? > + u32 reg_addr; > + u32 val; > +}; This struct is typecast straight from the byte array provided by request_firmware(), it would be preferable to use endian-specific data types to highlight this: i.e. presumably __le32 as written today. > + > +struct qcom_qps615_pwrctl_ctx { > + struct i2c_adapter *adapter; > + struct pci_pwrctl pwrctl; > + struct device_link *link; > + struct regulator *vdd; > +}; > + > +/* write 32-bit value to 24 bit register */ > +static int qps615_switch_i2c_write(struct qcom_qps615_pwrctl_ctx *ctx, u32 slv_addr, u32 reg_addr, > + u32 reg_val) addr, offset, val? > +{ > + struct i2c_msg msg; > + u8 msg_buf[7]; > + int ret; > + > + msg.addr = slv_addr; > + msg.len = 7; > + msg.flags = 0; > + > + /* Big Endian for reg addr */ > + msg_buf[0] = (u8)(reg_addr >> 16); > + msg_buf[1] = (u8)(reg_addr >> 8); > + msg_buf[2] = (u8)reg_addr; > + > + /* Little Endian for reg val */ > + msg_buf[3] = (u8)(reg_val); > + msg_buf[4] = (u8)(reg_val >> 8); > + msg_buf[5] = (u8)(reg_val >> 16); > + msg_buf[6] = (u8)(reg_val >> 24); > + > + msg.buf = msg_buf; > + ret = i2c_transfer(ctx->adapter, &msg, 1); Without qps615 being a child device of the adapter, does the kernel have adequate information to know that the geni I2C driver needs to be resumed before the qps615 driver, such that __i2c_transfer() will not bail as __i2c_check_suspended() reports that the bus is still suspended? > + return ret == 1 ? 0 : ret; > +} > + > +/* read 32 bit value from 24 bit reg addr */ > +static int qps615_switch_i2c_read(struct qcom_qps615_pwrctl_ctx *ctx, u32 slv_addr, u32 reg_addr, > + u32 *reg_val) > +{ > + u8 wr_data[3], rd_data[4] = {0}; > + struct i2c_msg msg[2]; > + int ret; > + > + msg[0].addr = slv_addr; > + msg[0].len = 3; > + msg[0].flags = 0; > + > + /* Big Endian for reg addr */ > + wr_data[0] = (u8)(reg_addr >> 16); > + wr_data[1] = (u8)(reg_addr >> 8); > + wr_data[2] = (u8)reg_addr; > + > + msg[0].buf = wr_data; > + > + msg[1].addr = slv_addr; > + msg[1].len = 4; > + msg[1].flags = I2C_M_RD; > + > + msg[1].buf = rd_data; > + > + ret = i2c_transfer(ctx->adapter, &msg[0], 2); > + if (ret != 2) > + return ret; > + > + *reg_val = (rd_data[3] << 24) | (rd_data[2] << 16) | (rd_data[1] << 8) | rd_data[0]; __le32 rd_data; msg[1].addr = &rd_data; msg[1].len = sizeof(rd_data); *value = le32_to_cpu(rd_data); Or do I get that backwards? > + > + return 0; > +} > + > +static int qcom_qps615_pwrctl_init(struct qcom_qps615_pwrctl_ctx *ctx) > +{ > + struct device *dev = ctx->pwrctl.dev; > + struct qcom_qps615_pwrctl_i2c_setting *set; > + const struct firmware *fw; > + const u8 *pos, *eof; > + int ret; > + u32 val; > + > + ret = request_firmware(&fw, "qcom/qps615.bin", dev); Is this the generic qps615 firmware, applicable to all implementations thereof, or does it contain device-specific calibration or such? > + if (ret < 0) { > + dev_err(dev, "firmware loading failed with ret %d\n", ret); > + return ret; > + } > + > + if (!fw) { > + ret = -EINVAL; > + goto err; > + } > + > + pos = fw->data; > + eof = fw->data + fw->size; > + > + while (pos < (fw->data + fw->size)) { In the case the firmware is truncated by 1 - 11 bytes this would read outside the buffer. Also, I presume this was the intended use for the now unused "eof" variable. > + set = (struct qcom_qps615_pwrctl_i2c_setting *)pos; I'd suggest that you treat fw->data as an array of struct qcom_qps615_pwrctl_i2c_setting elements. > + > + ret = qps615_switch_i2c_write(ctx, set->slv_addr, set->reg_addr, set->val); Is it possible to avoid getting the i2c device address from the firmware? Does/can this change? > + if (ret) { > + dev_err(dev, > + "I2c write failed for slv addr:%x at addr%x with val %x ret %d\n", Would be nice to clean this up a bit, there are no non-i2c accesses from this device, and the words "slv", "with", "val" has low SNR. Also %x does not communicate the base for many values, use %#x to get the 0x prefix. Perhaps: "failed to write %#x to #x on %#x\n", set->val, set->reg_addr, set->slv_addr > + set->slv_addr, set->reg_addr, set->val, ret); > + goto err; > + } > + > + ret = qps615_switch_i2c_read(ctx, set->slv_addr, set->reg_addr, &val); > + if (ret) { > + dev_err(dev, "I2c read failed for slv addr:%x at addr%x ret %d\n", "readback failed at %#x on %#x\n" > + set->slv_addr, set->reg_addr, ret); > + goto err; > + } > + > + if (set->val != val) { > + dev_err(dev, > + "I2c read's mismatch for slv:%x at addr%x exp%d got%d\n", "readback verification failed at %#x on %#x (%#x != %#x)\n" If there's only a single slv_addr value, then please omit that from all three error cases. > + set->slv_addr, set->reg_addr, set->val, val); > + goto err; > + } > + pos += sizeof(struct qcom_qps615_pwrctl_i2c_setting); > + } > + > +err: > + release_firmware(fw); > + > + return ret; > +} > + > +static int qcom_qps615_power_on(struct qcom_qps615_pwrctl_ctx *ctx) > +{ > + int ret; > + > + ret = regulator_enable(ctx->vdd); > + if (ret) { > + dev_err(ctx->pwrctl.dev, "cannot enable vdda regulator\n"); Is it vdd or vdda? > + return ret; > + } > + > + ret = qcom_qps615_pwrctl_init(ctx); > + if (ret) > + regulator_disable(ctx->vdd); > + > + return ret; > +} > + > +static int qcom_qps615_power_off(struct qcom_qps615_pwrctl_ctx *ctx) You don't care for the return value, so replace it with void. > +{ > + return regulator_disable(ctx->vdd); > +} > + > +static int qcom_qps615_pwrctl_probe(struct platform_device *pdev) > +{ > + struct device *dev = &pdev->dev; > + struct device_node *node = pdev->dev.of_node; > + struct qcom_qps615_pwrctl_ctx *ctx; > + struct device_node *adapter_node; > + struct pci_host_bridge *bridge; > + struct i2c_adapter *adapter; > + struct pci_bus *bus; > + > + bus = pci_find_bus(of_get_pci_domain_nr(dev->parent->of_node), 0); > + if (!bus) > + return -ENODEV; > + > + bridge = pci_find_host_bridge(bus); > + > + ctx = devm_kzalloc(dev, sizeof(*ctx), GFP_KERNEL); > + if (!ctx) > + return -ENOMEM; > + > + adapter_node = of_parse_phandle(node, "switch-i2c-cntrl", 0); > + if (adapter_node) { > + adapter = of_get_i2c_adapter_by_node(adapter_node); Per of_get_i2c_adapter_by_node() kernel-doc: "The user must call i2c_put_adapter(adapter) once done with the i2c adapter." > + __free(adapter_node); > + if (!adapter) > + return dev_err_probe(dev, -EPROBE_DEFER, > + "failed to parse switch-i2c-cntrl\n"); > + } > + > + ctx->pwrctl.dev = dev; > + ctx->adapter = adapter; > + ctx->vdd = devm_regulator_get(dev, "vdd"); > + if (IS_ERR(ctx->vdd)) > + return dev_err_probe(dev, PTR_ERR(ctx->vdd), > + "failed to get vdd regulator\n"); > + > + ctx->link = device_link_add(&bridge->dev, dev, DL_FLAG_AUTOREMOVE_CONSUMER); > + > + platform_set_drvdata(pdev, ctx); > + > + bridge->ops->stop_link(bus); > + qcom_qps615_power_on(ctx); > + bridge->ops->start_link(bus); > + > + return devm_pci_pwrctl_device_set_ready(dev, &ctx->pwrctl); > +} > + > +static const struct of_device_id qcom_qps615_pwrctl_of_match[] = { > + { > + .compatible = "pci1179,0623", > + }, > + { } > +}; > +MODULE_DEVICE_TABLE(of, qcom_qps615_pwrctl_of_match); This would fit better down by qcom_qps615_pwrctl_driver, instead of mixed in among the implementation. > + > +static int pci_pwrctl_suspend_noirq(struct device *dev) > +{ > + struct pci_bus *bus = pci_find_bus(of_get_pci_domain_nr(dev->parent->of_node), 0); > + struct pci_host_bridge *bridge = pci_find_host_bridge(bus); How about caching the bus and bridge in ctx? > + struct qcom_qps615_pwrctl_ctx *ctx = dev_get_drvdata(dev); > + > + bridge->ops->stop_link(bus); > + qcom_qps615_power_off(ctx); > + > + return 0; > +} > + > +static int pci_pwrctl_resume_noirq(struct device *dev) > +{ > + struct pci_bus *bus = pci_find_bus(of_get_pci_domain_nr(dev->parent->of_node), 0); > + struct pci_host_bridge *bridge = pci_find_host_bridge(bus); > + struct qcom_qps615_pwrctl_ctx *ctx = dev_get_drvdata(dev); > + > + qcom_qps615_power_on(ctx); Perhaps worth checking this return value? Although I guess there's not much to do if it's non-zero... Regards, Bjorn > + bridge->ops->start_link(bus); > + > + return 0; > +} > + > +static void qcom_qps615_pwrctl_remove(struct platform_device *pdev) > +{ > + struct device *dev = &pdev->dev; > + struct qcom_qps615_pwrctl_ctx *ctx = dev_get_drvdata(dev); > + > + device_link_del(ctx->link); > + pci_pwrctl_suspend_noirq(dev); > +} > + > +static const struct dev_pm_ops pci_pwrctl_pm_ops = { > + NOIRQ_SYSTEM_SLEEP_PM_OPS(pci_pwrctl_suspend_noirq, pci_pwrctl_resume_noirq) > +}; > + > +static struct platform_driver qcom_qps615_pwrctl_driver = { > + .driver = { > + .name = "pwrctl-qps615", > + .of_match_table = qcom_qps615_pwrctl_of_match, > + .pm = &pci_pwrctl_pm_ops, > + }, > + .probe = qcom_qps615_pwrctl_probe, > + .remove_new = qcom_qps615_pwrctl_remove, > +}; > +module_platform_driver(qcom_qps615_pwrctl_driver); > + > +MODULE_AUTHOR("Krishna chaitanya chundru <quic_krichai@quicinc.com>"); > +MODULE_DESCRIPTION("Qualcomm QPS615 power control driver"); > +MODULE_LICENSE("GPL"); > > -- > 2.42.0 >
On Wed, Jun 26, 2024 at 06:07:55PM GMT, Krishna chaitanya chundru wrote: > QPS615 switch needs to configured after powering on and before > PCIe link was up. > > As the PCIe controller driver already enables the PCIe link training > at the host side, stop the link training. > Otherwise the moment we turn on the switch it will participate in > the link training and link may come before switch is configured through > i2c. > > The switch can be configured different ways like changing de-emphasis > settings of the switch, disabling unused ports etc and these settings > can vary from board to board, for that reason the sequence is taken > from the firmware file which contains the address of the slave, to address > and data to be written to the switch. The driver reads the firmware file > and parses them to apply those configurations to the switch. > > Signed-off-by: Krishna chaitanya chundru <quic_krichai@quicinc.com> > --- > drivers/pci/pwrctl/Kconfig | 7 + > drivers/pci/pwrctl/Makefile | 1 + > drivers/pci/pwrctl/pci-pwrctl-qps615.c | 278 +++++++++++++++++++++++++++++++++ > 3 files changed, 286 insertions(+) > > diff --git a/drivers/pci/pwrctl/Kconfig b/drivers/pci/pwrctl/Kconfig > index f1b824955d4b..a419b250006d 100644 > --- a/drivers/pci/pwrctl/Kconfig > +++ b/drivers/pci/pwrctl/Kconfig > @@ -14,4 +14,11 @@ config PCI_PWRCTL_PWRSEQ > Enable support for the PCI power control driver for device > drivers using the Power Sequencing subsystem. > > +config PCI_PWRCTL_QPS615 > + tristate "PCI Power Control driver for QPS615" > + select PCI_PWRCTL > + help > + Enable support for the PCI power control driver for QPS615 and > + configures it. > + > endmenu > diff --git a/drivers/pci/pwrctl/Makefile b/drivers/pci/pwrctl/Makefile > index d308aae4800c..ac563a70c023 100644 > --- a/drivers/pci/pwrctl/Makefile > +++ b/drivers/pci/pwrctl/Makefile > @@ -4,3 +4,4 @@ obj-$(CONFIG_PCI_PWRCTL) += pci-pwrctl-core.o > pci-pwrctl-core-y := core.o > > obj-$(CONFIG_PCI_PWRCTL_PWRSEQ) += pci-pwrctl-pwrseq.o > +obj-$(CONFIG_PCI_PWRCTL_QPS615) += pci-pwrctl-qps615.o > diff --git a/drivers/pci/pwrctl/pci-pwrctl-qps615.c b/drivers/pci/pwrctl/pci-pwrctl-qps615.c > new file mode 100644 > index 000000000000..1f2caf5d7da2 > --- /dev/null > +++ b/drivers/pci/pwrctl/pci-pwrctl-qps615.c > @@ -0,0 +1,278 @@ > +// SPDX-License-Identifier: GPL-2.0-only > +/* > + * Copyright (c) 2023 Qualcomm Innovation Center, Inc. All rights reserved. > + */ > + > +#include <linux/device.h> > +#include <linux/firmware.h> > +#include <linux/i2c.h> > +#include <linux/mod_devicetable.h> > +#include <linux/module.h> > +#include <linux/of.h> > +#include <linux/pci-pwrctl.h> > +#include <linux/platform_device.h> > +#include <linux/regulator/consumer.h> > +#include <linux/string.h> > +#include <linux/types.h> > + > +#include "../pci.h" > + > +struct qcom_qps615_pwrctl_i2c_setting { > + u32 slv_addr; > + u32 reg_addr; > + u32 val; > +}; > + > +struct qcom_qps615_pwrctl_ctx { > + struct i2c_adapter *adapter; > + struct pci_pwrctl pwrctl; > + struct device_link *link; > + struct regulator *vdd; > +}; > + > +/* write 32-bit value to 24 bit register */ > +static int qps615_switch_i2c_write(struct qcom_qps615_pwrctl_ctx *ctx, u32 slv_addr, u32 reg_addr, > + u32 reg_val) > +{ > + struct i2c_msg msg; > + u8 msg_buf[7]; > + int ret; > + > + msg.addr = slv_addr; > + msg.len = 7; > + msg.flags = 0; > + > + /* Big Endian for reg addr */ > + msg_buf[0] = (u8)(reg_addr >> 16); > + msg_buf[1] = (u8)(reg_addr >> 8); > + msg_buf[2] = (u8)reg_addr; > + > + /* Little Endian for reg val */ > + msg_buf[3] = (u8)(reg_val); > + msg_buf[4] = (u8)(reg_val >> 8); > + msg_buf[5] = (u8)(reg_val >> 16); > + msg_buf[6] = (u8)(reg_val >> 24); > + > + msg.buf = msg_buf; > + ret = i2c_transfer(ctx->adapter, &msg, 1); > + return ret == 1 ? 0 : ret; > +} > + > +/* read 32 bit value from 24 bit reg addr */ > +static int qps615_switch_i2c_read(struct qcom_qps615_pwrctl_ctx *ctx, u32 slv_addr, u32 reg_addr, > + u32 *reg_val) > +{ > + u8 wr_data[3], rd_data[4] = {0}; > + struct i2c_msg msg[2]; > + int ret; > + > + msg[0].addr = slv_addr; > + msg[0].len = 3; > + msg[0].flags = 0; > + > + /* Big Endian for reg addr */ > + wr_data[0] = (u8)(reg_addr >> 16); > + wr_data[1] = (u8)(reg_addr >> 8); > + wr_data[2] = (u8)reg_addr; > + > + msg[0].buf = wr_data; > + > + msg[1].addr = slv_addr; > + msg[1].len = 4; > + msg[1].flags = I2C_M_RD; > + > + msg[1].buf = rd_data; > + > + ret = i2c_transfer(ctx->adapter, &msg[0], 2); > + if (ret != 2) > + return ret; > + > + *reg_val = (rd_data[3] << 24) | (rd_data[2] << 16) | (rd_data[1] << 8) | rd_data[0]; > + > + return 0; > +} > + > +static int qcom_qps615_pwrctl_init(struct qcom_qps615_pwrctl_ctx *ctx) > +{ > + struct device *dev = ctx->pwrctl.dev; > + struct qcom_qps615_pwrctl_i2c_setting *set; > + const struct firmware *fw; > + const u8 *pos, *eof; > + int ret; > + u32 val; > + > + ret = request_firmware(&fw, "qcom/qps615.bin", dev); Oh, what if any other board uses the same bridge? Do you expect that the rootfs is device-specific? > + if (ret < 0) { > + dev_err(dev, "firmware loading failed with ret %d\n", ret); > + return ret; > + } > + > + if (!fw) { > + ret = -EINVAL; > + goto err; > + } > + > + pos = fw->data; > + eof = fw->data + fw->size; > + > + while (pos < (fw->data + fw->size)) { > + set = (struct qcom_qps615_pwrctl_i2c_setting *)pos; > + > + ret = qps615_switch_i2c_write(ctx, set->slv_addr, set->reg_addr, set->val); No! This makes no way for anybody to understand what is going on. Please write a proper driver, that defines registers and writes sensible data to those registers. Having a driver that writes data from the configuration file to the random registers is a no-go. > + if (ret) { > + dev_err(dev, > + "I2c write failed for slv addr:%x at addr%x with val %x ret %d\n", > + set->slv_addr, set->reg_addr, set->val, ret); > + goto err; > + } > + > + ret = qps615_switch_i2c_read(ctx, set->slv_addr, set->reg_addr, &val); > + if (ret) { > + dev_err(dev, "I2c read failed for slv addr:%x at addr%x ret %d\n", > + set->slv_addr, set->reg_addr, ret); > + goto err; > + } > + > + if (set->val != val) { > + dev_err(dev, > + "I2c read's mismatch for slv:%x at addr%x exp%d got%d\n", > + set->slv_addr, set->reg_addr, set->val, val); > + goto err; > + } > + pos += sizeof(struct qcom_qps615_pwrctl_i2c_setting); > + } > + > +err: > + release_firmware(fw); > + > + return ret; > +} > + > +static int qcom_qps615_power_on(struct qcom_qps615_pwrctl_ctx *ctx) > +{ > + int ret; > + > + ret = regulator_enable(ctx->vdd); > + if (ret) { > + dev_err(ctx->pwrctl.dev, "cannot enable vdda regulator\n"); > + return ret; > + } > + > + ret = qcom_qps615_pwrctl_init(ctx); > + if (ret) > + regulator_disable(ctx->vdd); > + > + return ret; > +} > + > +static int qcom_qps615_power_off(struct qcom_qps615_pwrctl_ctx *ctx) > +{ > + return regulator_disable(ctx->vdd); > +} > + > +static int qcom_qps615_pwrctl_probe(struct platform_device *pdev) > +{ > + struct device *dev = &pdev->dev; > + struct device_node *node = pdev->dev.of_node; > + struct qcom_qps615_pwrctl_ctx *ctx; > + struct device_node *adapter_node; > + struct pci_host_bridge *bridge; > + struct i2c_adapter *adapter; > + struct pci_bus *bus; > + > + bus = pci_find_bus(of_get_pci_domain_nr(dev->parent->of_node), 0); > + if (!bus) > + return -ENODEV; > + > + bridge = pci_find_host_bridge(bus); > + > + ctx = devm_kzalloc(dev, sizeof(*ctx), GFP_KERNEL); > + if (!ctx) > + return -ENOMEM; > + > + adapter_node = of_parse_phandle(node, "switch-i2c-cntrl", 0); > + if (adapter_node) { > + adapter = of_get_i2c_adapter_by_node(adapter_node); Somebody didn't read the comment before of_get_i2c_adapter_by_node(). > + __free(adapter_node); > + if (!adapter) > + return dev_err_probe(dev, -EPROBE_DEFER, > + "failed to parse switch-i2c-cntrl\n"); > + } > + > + ctx->pwrctl.dev = dev; > + ctx->adapter = adapter; > + ctx->vdd = devm_regulator_get(dev, "vdd"); > + if (IS_ERR(ctx->vdd)) > + return dev_err_probe(dev, PTR_ERR(ctx->vdd), > + "failed to get vdd regulator\n"); > + > + ctx->link = device_link_add(&bridge->dev, dev, DL_FLAG_AUTOREMOVE_CONSUMER); > + > + platform_set_drvdata(pdev, ctx); > + > + bridge->ops->stop_link(bus); > + qcom_qps615_power_on(ctx); > + bridge->ops->start_link(bus); > + > + return devm_pci_pwrctl_device_set_ready(dev, &ctx->pwrctl); > +} I'd suggest turning this into a driver which uses components to bind the PCIe power control part and the i2c clinet device. Then you can write the i2c client addresses as ussual. > + > +static const struct of_device_id qcom_qps615_pwrctl_of_match[] = { > + { > + .compatible = "pci1179,0623", > + }, > + { } > +}; > +MODULE_DEVICE_TABLE(of, qcom_qps615_pwrctl_of_match); > + > +static int pci_pwrctl_suspend_noirq(struct device *dev) > +{ > + struct pci_bus *bus = pci_find_bus(of_get_pci_domain_nr(dev->parent->of_node), 0); > + struct pci_host_bridge *bridge = pci_find_host_bridge(bus); > + struct qcom_qps615_pwrctl_ctx *ctx = dev_get_drvdata(dev); > + > + bridge->ops->stop_link(bus); > + qcom_qps615_power_off(ctx); > + > + return 0; > +} > + > +static int pci_pwrctl_resume_noirq(struct device *dev) > +{ > + struct pci_bus *bus = pci_find_bus(of_get_pci_domain_nr(dev->parent->of_node), 0); > + struct pci_host_bridge *bridge = pci_find_host_bridge(bus); > + struct qcom_qps615_pwrctl_ctx *ctx = dev_get_drvdata(dev); > + > + qcom_qps615_power_on(ctx); > + bridge->ops->start_link(bus); > + > + return 0; > +} > + > +static void qcom_qps615_pwrctl_remove(struct platform_device *pdev) > +{ > + struct device *dev = &pdev->dev; > + struct qcom_qps615_pwrctl_ctx *ctx = dev_get_drvdata(dev); > + > + device_link_del(ctx->link); > + pci_pwrctl_suspend_noirq(dev); > +} > + > +static const struct dev_pm_ops pci_pwrctl_pm_ops = { > + NOIRQ_SYSTEM_SLEEP_PM_OPS(pci_pwrctl_suspend_noirq, pci_pwrctl_resume_noirq) > +}; > + > +static struct platform_driver qcom_qps615_pwrctl_driver = { > + .driver = { > + .name = "pwrctl-qps615", > + .of_match_table = qcom_qps615_pwrctl_of_match, > + .pm = &pci_pwrctl_pm_ops, > + }, > + .probe = qcom_qps615_pwrctl_probe, > + .remove_new = qcom_qps615_pwrctl_remove, > +}; > +module_platform_driver(qcom_qps615_pwrctl_driver); > + > +MODULE_AUTHOR("Krishna chaitanya chundru <quic_krichai@quicinc.com>"); > +MODULE_DESCRIPTION("Qualcomm QPS615 power control driver"); > +MODULE_LICENSE("GPL"); > > -- > 2.42.0 >
On Wed, Jun 26, 2024 at 06:07:55PM +0530, Krishna chaitanya chundru wrote: > QPS615 switch needs to configured after powering on and before > PCIe link was up. s/need to configured/needs to be configured/ > As the PCIe controller driver already enables the PCIe link training > at the host side, stop the link training. Add blank line between paragraphs or rewrap into a single paragraph. > Otherwise the moment we turn on the switch it will participate in > the link training and link may come before switch is configured through > i2c. s/link may come before/link may come up before/ ? > The switch can be configured different ways like changing de-emphasis > settings of the switch, disabling unused ports etc and these settings > can vary from board to board, for that reason the sequence is taken > from the firmware file which contains the address of the slave, to address > and data to be written to the switch. The driver reads the firmware file > and parses them to apply those configurations to the switch. s/to address and data/the address and the data/ ? s/and parses them/and parses it/ (I assume you parse the file?)
On 6/26/2024 9:07 PM, Konrad Dybcio wrote: > On 26.06.2024 2:37 PM, Krishna chaitanya chundru wrote: >> QPS615 switch needs to configured after powering on and before >> PCIe link was up. >> >> As the PCIe controller driver already enables the PCIe link training >> at the host side, stop the link training. >> Otherwise the moment we turn on the switch it will participate in >> the link training and link may come before switch is configured through >> i2c. >> >> The switch can be configured different ways like changing de-emphasis >> settings of the switch, disabling unused ports etc and these settings >> can vary from board to board, for that reason the sequence is taken >> from the firmware file which contains the address of the slave, to address >> and data to be written to the switch. The driver reads the firmware file >> and parses them to apply those configurations to the switch. >> >> Signed-off-by: Krishna chaitanya chundru <quic_krichai@quicinc.com> >> --- > > [...] > >> +static int qcom_qps615_pwrctl_init(struct qcom_qps615_pwrctl_ctx *ctx) >> +{ >> + struct device *dev = ctx->pwrctl.dev; >> + struct qcom_qps615_pwrctl_i2c_setting *set; >> + const struct firmware *fw; >> + const u8 *pos, *eof; >> + int ret; >> + u32 val; >> + >> + ret = request_firmware(&fw, "qcom/qps615.bin", dev); > > Is this driver only going to serve one model of the device, that will use > this specific firmware file, ever? > > In other words, is QPS615 super special and no other chip like it will be > ever made? > > [...] > >> + >> + bridge->ops->stop_link(bus); > > This is turbo intrusive. What if there are more devices on this bus? > The expectation of this API from the controller driver is to stop link training only when the link is not up. - krishna chaitanya. > Konrad
diff --git a/drivers/pci/pwrctl/Kconfig b/drivers/pci/pwrctl/Kconfig index f1b824955d4b..a419b250006d 100644 --- a/drivers/pci/pwrctl/Kconfig +++ b/drivers/pci/pwrctl/Kconfig @@ -14,4 +14,11 @@ config PCI_PWRCTL_PWRSEQ Enable support for the PCI power control driver for device drivers using the Power Sequencing subsystem. +config PCI_PWRCTL_QPS615 + tristate "PCI Power Control driver for QPS615" + select PCI_PWRCTL + help + Enable support for the PCI power control driver for QPS615 and + configures it. + endmenu diff --git a/drivers/pci/pwrctl/Makefile b/drivers/pci/pwrctl/Makefile index d308aae4800c..ac563a70c023 100644 --- a/drivers/pci/pwrctl/Makefile +++ b/drivers/pci/pwrctl/Makefile @@ -4,3 +4,4 @@ obj-$(CONFIG_PCI_PWRCTL) += pci-pwrctl-core.o pci-pwrctl-core-y := core.o obj-$(CONFIG_PCI_PWRCTL_PWRSEQ) += pci-pwrctl-pwrseq.o +obj-$(CONFIG_PCI_PWRCTL_QPS615) += pci-pwrctl-qps615.o diff --git a/drivers/pci/pwrctl/pci-pwrctl-qps615.c b/drivers/pci/pwrctl/pci-pwrctl-qps615.c new file mode 100644 index 000000000000..1f2caf5d7da2 --- /dev/null +++ b/drivers/pci/pwrctl/pci-pwrctl-qps615.c @@ -0,0 +1,278 @@ +// SPDX-License-Identifier: GPL-2.0-only +/* + * Copyright (c) 2023 Qualcomm Innovation Center, Inc. All rights reserved. + */ + +#include <linux/device.h> +#include <linux/firmware.h> +#include <linux/i2c.h> +#include <linux/mod_devicetable.h> +#include <linux/module.h> +#include <linux/of.h> +#include <linux/pci-pwrctl.h> +#include <linux/platform_device.h> +#include <linux/regulator/consumer.h> +#include <linux/string.h> +#include <linux/types.h> + +#include "../pci.h" + +struct qcom_qps615_pwrctl_i2c_setting { + u32 slv_addr; + u32 reg_addr; + u32 val; +}; + +struct qcom_qps615_pwrctl_ctx { + struct i2c_adapter *adapter; + struct pci_pwrctl pwrctl; + struct device_link *link; + struct regulator *vdd; +}; + +/* write 32-bit value to 24 bit register */ +static int qps615_switch_i2c_write(struct qcom_qps615_pwrctl_ctx *ctx, u32 slv_addr, u32 reg_addr, + u32 reg_val) +{ + struct i2c_msg msg; + u8 msg_buf[7]; + int ret; + + msg.addr = slv_addr; + msg.len = 7; + msg.flags = 0; + + /* Big Endian for reg addr */ + msg_buf[0] = (u8)(reg_addr >> 16); + msg_buf[1] = (u8)(reg_addr >> 8); + msg_buf[2] = (u8)reg_addr; + + /* Little Endian for reg val */ + msg_buf[3] = (u8)(reg_val); + msg_buf[4] = (u8)(reg_val >> 8); + msg_buf[5] = (u8)(reg_val >> 16); + msg_buf[6] = (u8)(reg_val >> 24); + + msg.buf = msg_buf; + ret = i2c_transfer(ctx->adapter, &msg, 1); + return ret == 1 ? 0 : ret; +} + +/* read 32 bit value from 24 bit reg addr */ +static int qps615_switch_i2c_read(struct qcom_qps615_pwrctl_ctx *ctx, u32 slv_addr, u32 reg_addr, + u32 *reg_val) +{ + u8 wr_data[3], rd_data[4] = {0}; + struct i2c_msg msg[2]; + int ret; + + msg[0].addr = slv_addr; + msg[0].len = 3; + msg[0].flags = 0; + + /* Big Endian for reg addr */ + wr_data[0] = (u8)(reg_addr >> 16); + wr_data[1] = (u8)(reg_addr >> 8); + wr_data[2] = (u8)reg_addr; + + msg[0].buf = wr_data; + + msg[1].addr = slv_addr; + msg[1].len = 4; + msg[1].flags = I2C_M_RD; + + msg[1].buf = rd_data; + + ret = i2c_transfer(ctx->adapter, &msg[0], 2); + if (ret != 2) + return ret; + + *reg_val = (rd_data[3] << 24) | (rd_data[2] << 16) | (rd_data[1] << 8) | rd_data[0]; + + return 0; +} + +static int qcom_qps615_pwrctl_init(struct qcom_qps615_pwrctl_ctx *ctx) +{ + struct device *dev = ctx->pwrctl.dev; + struct qcom_qps615_pwrctl_i2c_setting *set; + const struct firmware *fw; + const u8 *pos, *eof; + int ret; + u32 val; + + ret = request_firmware(&fw, "qcom/qps615.bin", dev); + if (ret < 0) { + dev_err(dev, "firmware loading failed with ret %d\n", ret); + return ret; + } + + if (!fw) { + ret = -EINVAL; + goto err; + } + + pos = fw->data; + eof = fw->data + fw->size; + + while (pos < (fw->data + fw->size)) { + set = (struct qcom_qps615_pwrctl_i2c_setting *)pos; + + ret = qps615_switch_i2c_write(ctx, set->slv_addr, set->reg_addr, set->val); + if (ret) { + dev_err(dev, + "I2c write failed for slv addr:%x at addr%x with val %x ret %d\n", + set->slv_addr, set->reg_addr, set->val, ret); + goto err; + } + + ret = qps615_switch_i2c_read(ctx, set->slv_addr, set->reg_addr, &val); + if (ret) { + dev_err(dev, "I2c read failed for slv addr:%x at addr%x ret %d\n", + set->slv_addr, set->reg_addr, ret); + goto err; + } + + if (set->val != val) { + dev_err(dev, + "I2c read's mismatch for slv:%x at addr%x exp%d got%d\n", + set->slv_addr, set->reg_addr, set->val, val); + goto err; + } + pos += sizeof(struct qcom_qps615_pwrctl_i2c_setting); + } + +err: + release_firmware(fw); + + return ret; +} + +static int qcom_qps615_power_on(struct qcom_qps615_pwrctl_ctx *ctx) +{ + int ret; + + ret = regulator_enable(ctx->vdd); + if (ret) { + dev_err(ctx->pwrctl.dev, "cannot enable vdda regulator\n"); + return ret; + } + + ret = qcom_qps615_pwrctl_init(ctx); + if (ret) + regulator_disable(ctx->vdd); + + return ret; +} + +static int qcom_qps615_power_off(struct qcom_qps615_pwrctl_ctx *ctx) +{ + return regulator_disable(ctx->vdd); +} + +static int qcom_qps615_pwrctl_probe(struct platform_device *pdev) +{ + struct device *dev = &pdev->dev; + struct device_node *node = pdev->dev.of_node; + struct qcom_qps615_pwrctl_ctx *ctx; + struct device_node *adapter_node; + struct pci_host_bridge *bridge; + struct i2c_adapter *adapter; + struct pci_bus *bus; + + bus = pci_find_bus(of_get_pci_domain_nr(dev->parent->of_node), 0); + if (!bus) + return -ENODEV; + + bridge = pci_find_host_bridge(bus); + + ctx = devm_kzalloc(dev, sizeof(*ctx), GFP_KERNEL); + if (!ctx) + return -ENOMEM; + + adapter_node = of_parse_phandle(node, "switch-i2c-cntrl", 0); + if (adapter_node) { + adapter = of_get_i2c_adapter_by_node(adapter_node); + __free(adapter_node); + if (!adapter) + return dev_err_probe(dev, -EPROBE_DEFER, + "failed to parse switch-i2c-cntrl\n"); + } + + ctx->pwrctl.dev = dev; + ctx->adapter = adapter; + ctx->vdd = devm_regulator_get(dev, "vdd"); + if (IS_ERR(ctx->vdd)) + return dev_err_probe(dev, PTR_ERR(ctx->vdd), + "failed to get vdd regulator\n"); + + ctx->link = device_link_add(&bridge->dev, dev, DL_FLAG_AUTOREMOVE_CONSUMER); + + platform_set_drvdata(pdev, ctx); + + bridge->ops->stop_link(bus); + qcom_qps615_power_on(ctx); + bridge->ops->start_link(bus); + + return devm_pci_pwrctl_device_set_ready(dev, &ctx->pwrctl); +} + +static const struct of_device_id qcom_qps615_pwrctl_of_match[] = { + { + .compatible = "pci1179,0623", + }, + { } +}; +MODULE_DEVICE_TABLE(of, qcom_qps615_pwrctl_of_match); + +static int pci_pwrctl_suspend_noirq(struct device *dev) +{ + struct pci_bus *bus = pci_find_bus(of_get_pci_domain_nr(dev->parent->of_node), 0); + struct pci_host_bridge *bridge = pci_find_host_bridge(bus); + struct qcom_qps615_pwrctl_ctx *ctx = dev_get_drvdata(dev); + + bridge->ops->stop_link(bus); + qcom_qps615_power_off(ctx); + + return 0; +} + +static int pci_pwrctl_resume_noirq(struct device *dev) +{ + struct pci_bus *bus = pci_find_bus(of_get_pci_domain_nr(dev->parent->of_node), 0); + struct pci_host_bridge *bridge = pci_find_host_bridge(bus); + struct qcom_qps615_pwrctl_ctx *ctx = dev_get_drvdata(dev); + + qcom_qps615_power_on(ctx); + bridge->ops->start_link(bus); + + return 0; +} + +static void qcom_qps615_pwrctl_remove(struct platform_device *pdev) +{ + struct device *dev = &pdev->dev; + struct qcom_qps615_pwrctl_ctx *ctx = dev_get_drvdata(dev); + + device_link_del(ctx->link); + pci_pwrctl_suspend_noirq(dev); +} + +static const struct dev_pm_ops pci_pwrctl_pm_ops = { + NOIRQ_SYSTEM_SLEEP_PM_OPS(pci_pwrctl_suspend_noirq, pci_pwrctl_resume_noirq) +}; + +static struct platform_driver qcom_qps615_pwrctl_driver = { + .driver = { + .name = "pwrctl-qps615", + .of_match_table = qcom_qps615_pwrctl_of_match, + .pm = &pci_pwrctl_pm_ops, + }, + .probe = qcom_qps615_pwrctl_probe, + .remove_new = qcom_qps615_pwrctl_remove, +}; +module_platform_driver(qcom_qps615_pwrctl_driver); + +MODULE_AUTHOR("Krishna chaitanya chundru <quic_krichai@quicinc.com>"); +MODULE_DESCRIPTION("Qualcomm QPS615 power control driver"); +MODULE_LICENSE("GPL");
QPS615 switch needs to configured after powering on and before PCIe link was up. As the PCIe controller driver already enables the PCIe link training at the host side, stop the link training. Otherwise the moment we turn on the switch it will participate in the link training and link may come before switch is configured through i2c. The switch can be configured different ways like changing de-emphasis settings of the switch, disabling unused ports etc and these settings can vary from board to board, for that reason the sequence is taken from the firmware file which contains the address of the slave, to address and data to be written to the switch. The driver reads the firmware file and parses them to apply those configurations to the switch. Signed-off-by: Krishna chaitanya chundru <quic_krichai@quicinc.com> --- drivers/pci/pwrctl/Kconfig | 7 + drivers/pci/pwrctl/Makefile | 1 + drivers/pci/pwrctl/pci-pwrctl-qps615.c | 278 +++++++++++++++++++++++++++++++++ 3 files changed, 286 insertions(+)