| Message ID | 20240520074300.125969-5-yoshihiro.shimoda.uh@renesas.com (mailing list archive) |
|---|---|
| State | Superseded |
| Delegated to: | Geert Uytterhoeven |
| Headers | show |
| Series | PCI: rcar-gen4: Add R-Car V4H support | expand |
On Mon, May 20, 2024 at 04:42:59PM +0900, Yoshihiro Shimoda wrote: > Add support for r8a779g0 (R-Car V4H). > > This driver previously supported r8a779f0 (R-Car S4-8). PCIe features > of both r8a779f0 and r8a779g0 are almost all the same. For example: > - PCI Express Base Specification Revision 4.0 > - Root complex mode and endpoint mode are supported > However, r8a779g0 requires specific firmware downloading, to > initialize the PHY. Otherwise, the PCIe controller cannot work. > > The attached firmware file "104_PCIe_fw_addr_data_ver1.05.txt" in > the manual is a text file. But, Renesas is not able to distribute > the firmware freely. So, we require converting the text file > to a binary before the driver runs by using the following script: > > $ awk '/^\s*0x[0-9A-Fa-f]{4}\s+0x[0-9A-Fa-f]{4}/ \ > { print substr($2,5,2) substr($2,3,2) }' \ > 104_PCIe_fw_addr_data_ver1.05.txt | xxd -p -r > \ > rcar_gen4_pcie.bin > $ sha1sum rcar_gen4_pcie.bin > 1d0bd4b189b4eb009f5d564b1f93a79112994945 rcar_gen4_pcie.bin > > Signed-off-by: Yoshihiro Shimoda <yoshihiro.shimoda.uh@renesas.com> > --- > drivers/pci/controller/dwc/pcie-rcar-gen4.c | 194 +++++++++++++++++++- > 1 file changed, 193 insertions(+), 1 deletion(-) > > diff --git a/drivers/pci/controller/dwc/pcie-rcar-gen4.c b/drivers/pci/controller/dwc/pcie-rcar-gen4.c > index bcbf0a52890d..f766a9739e15 100644 > --- a/drivers/pci/controller/dwc/pcie-rcar-gen4.c > +++ b/drivers/pci/controller/dwc/pcie-rcar-gen4.c > @@ -5,8 +5,10 @@ > */ > > #include <linux/delay.h> > +#include <linux/firmware.h> > #include <linux/interrupt.h> > #include <linux/io.h> > +#include <linux/iopoll.h> > #include <linux/module.h> > #include <linux/of.h> > #include <linux/pci.h> > @@ -20,9 +22,10 @@ > /* Renesas-specific */ > /* PCIe Mode Setting Register 0 */ > #define PCIEMSR0 0x0000 > -#define BIFUR_MOD_SET_ON BIT(0) > +#define APP_SRIS_MODE BIT(6) > #define DEVICE_TYPE_EP 0 > #define DEVICE_TYPE_RC BIT(4) > +#define BIFUR_MOD_SET_ON BIT(0) > > /* PCIe Interrupt Status 0 */ > #define PCIEINTSTS0 0x0084 > @@ -37,19 +40,36 @@ > #define PCIEDMAINTSTSEN 0x0314 > #define PCIEDMAINTSTSEN_INIT GENMASK(15, 0) > > +/* Port Logic Registers 89 */ > +#define PRTLGC89 0x0b70 > + > +/* Port Logic Registers 90 */ > +#define PRTLGC90 0x0b74 > + > /* PCIe Reset Control Register 1 */ > #define PCIERSTCTRL1 0x0014 > #define APP_HOLD_PHY_RST BIT(16) > #define APP_LTSSM_ENABLE BIT(0) > > +/* PCIe Power Management Control */ > +#define PCIEPWRMNGCTRL 0x0070 > +#define APP_CLK_REQ_N BIT(11) > +#define APP_CLK_PM_EN BIT(10) > + > #define RCAR_NUM_SPEED_CHANGE_RETRIES 10 > #define RCAR_MAX_LINK_SPEED 4 > > #define RCAR_GEN4_PCIE_EP_FUNC_DBI_OFFSET 0x1000 > #define RCAR_GEN4_PCIE_EP_FUNC_DBI2_OFFSET 0x800 > > +/* About the firmware, please refer to the commit log */ Which commit log? This is no useful info to the developers. Please add the actual information about the firmware here (whatever you mentioned in the commit message). > +#define RCAR_GEN4_PCIE_FIRMWARE_NAME "rcar_gen4_pcie.bin" > +#define RCAR_GEN4_PCIE_FIRMWARE_BASE_ADDR 0xc000 > +MODULE_FIRMWARE(RCAR_GEN4_PCIE_FIRMWARE_NAME); > + > struct rcar_gen4_pcie; > struct rcar_gen4_pcie_drvdata { > + void (*additional_common_init)(struct rcar_gen4_pcie *rcar); > int (*ltssm_control)(struct rcar_gen4_pcie *rcar, bool enable); > enum dw_pcie_device_mode mode; > }; > @@ -57,6 +77,7 @@ struct rcar_gen4_pcie_drvdata { > struct rcar_gen4_pcie { > struct dw_pcie dw; > void __iomem *base; > + void __iomem *phy_base; > struct platform_device *pdev; > const struct rcar_gen4_pcie_drvdata *drvdata; > }; > @@ -180,6 +201,9 @@ static int rcar_gen4_pcie_common_init(struct rcar_gen4_pcie *rcar) > if (ret) > goto err_unprepare; > > + if (rcar->drvdata->additional_common_init) > + rcar->drvdata->additional_common_init(rcar); > + > return 0; > > err_unprepare: > @@ -221,6 +245,10 @@ static void rcar_gen4_pcie_unprepare(struct rcar_gen4_pcie *rcar) > > static int rcar_gen4_pcie_get_resources(struct rcar_gen4_pcie *rcar) > { > + rcar->phy_base = devm_platform_ioremap_resource_byname(rcar->pdev, "phy"); > + if (IS_ERR(rcar->phy_base)) > + return PTR_ERR(rcar->phy_base); > + > /* Renesas-specific registers */ > rcar->base = devm_platform_ioremap_resource_byname(rcar->pdev, "app"); > > @@ -514,6 +542,166 @@ static int r8a779f0_pcie_ltssm_control(struct rcar_gen4_pcie *rcar, bool enable) > return 0; > } > > +static void rcar_gen4_pcie_additional_common_init(struct rcar_gen4_pcie *rcar) > +{ > + struct dw_pcie *dw = &rcar->dw; > + u32 val; > + > + /* > + * The SoC manual said the register setting is required. Otherwise, > + * linkup failed. This also can be dropped. > + */ > + val = dw_pcie_readl_dbi(dw, PCIE_PORT_LANE_SKEW); > + val &= ~PORT_LANE_SKEW_INSERT_MASK; > + if (dw->num_lanes < 4) > + val |= BIT(6); > + dw_pcie_writel_dbi(dw, PCIE_PORT_LANE_SKEW, val); > + > + val = readl(rcar->base + PCIEPWRMNGCTRL); > + val |= APP_CLK_REQ_N | APP_CLK_PM_EN; > + writel(val, rcar->base + PCIEPWRMNGCTRL); > +} > + > +static void rcar_gen4_pcie_phy_reg_update_bits(struct rcar_gen4_pcie *rcar, > + u32 offset, u32 mask, u32 val) > +{ > + u32 tmp; > + > + tmp = readl(rcar->phy_base + offset); > + tmp &= ~mask; > + tmp |= val; Use FIELD_* macros to avoid using the shift value. > + writel(tmp, rcar->phy_base + offset); > +} > + > +static int rcar_gen4_pcie_reg_test_bit(struct rcar_gen4_pcie *rcar, > + u32 offset, u32 mask) > +{ > + struct dw_pcie *dw = &rcar->dw; > + > + if (dw_pcie_readl_dbi(dw, offset) & mask) > + return -EAGAIN; Here a comment like below could be added at the top of the function definition. /* * SoC reference manual suggests checking port logic register bits * during firmware write. If read returns non-zero value, then this * function returns -EAGAIN indicating that the write needs to be done * again. If read returns zero, then return 0 to indicate success. */ > + > + return 0; > +} > + > +static int rcar_gen4_pcie_download_phy_firmware(struct rcar_gen4_pcie *rcar) > +{ > + /* The check_addr values are magical numbers in the datasheet */ > + const u32 check_addr[] = { 0x00101018, 0x00101118, 0x00101021, 0x00101121}; > + struct dw_pcie *dw = &rcar->dw; > + const struct firmware *fw; > + unsigned int i, timeout; > + u32 data; > + int ret; > + > + ret = request_firmware(&fw, RCAR_GEN4_PCIE_FIRMWARE_NAME, dw->dev); > + if (ret) { > + dev_err(dw->dev, "%s: Requesting firmware failed (%s)\n", > + __func__, RCAR_GEN4_PCIE_FIRMWARE_NAME); dev_err(dw->dev, "Failed to load firmware (%s): %d", RCAR_GEN4_PCIE_FIRMWARE_NAME, ret); > + return ret; > + } > + > + for (i = 0; i < (fw->size / 2); i++) { > + data = fw->data[(i * 2) + 1] << 8 | fw->data[i * 2]; > + timeout = 100; > + do { > + dw_pcie_writel_dbi(dw, PRTLGC89, RCAR_GEN4_PCIE_FIRMWARE_BASE_ADDR + i); > + dw_pcie_writel_dbi(dw, PRTLGC90, data); > + if (rcar_gen4_pcie_reg_test_bit(rcar, PRTLGC89, BIT(30)) >= 0) Can the return value > 0? > + break; > + if (!(--timeout)) { > + ret = -ETIMEDOUT; > + goto exit; > + } > + usleep_range(100, 200); > + } while (1); > + } > + > + rcar_gen4_pcie_phy_reg_update_bits(rcar, 0x0f8, BIT(17), BIT(17)); > + > + for (i = 0; i < ARRAY_SIZE(check_addr); i++) { > + timeout = 100; > + do { > + dw_pcie_writel_dbi(dw, PRTLGC89, check_addr[i]); > + ret = rcar_gen4_pcie_reg_test_bit(rcar, PRTLGC89, BIT(30)); > + ret |= rcar_gen4_pcie_reg_test_bit(rcar, PRTLGC90, BIT(0)); > + if (ret >= 0) Same as above. > + break; > + if (!(--timeout)) { > + ret = -ETIMEDOUT; > + goto exit; > + } > + usleep_range(100, 200); > + } while (1); > + } > + > + ret = 0; At this point, ret should be 0, right? > +exit: > + release_firmware(fw); > + > + return ret; > +} > + > +static int rcar_gen4_pcie_ltssm_control(struct rcar_gen4_pcie *rcar, bool enable) > +{ > + struct dw_pcie *dw = &rcar->dw; > + u32 val; > + int ret; > + > + if (!enable) { > + val = readl(rcar->base + PCIERSTCTRL1); There is no need to use 'readl/writel' here (which has the barrier), but since this driver is already using them, I'm not asking to change it now. But please consider switching to _relaxed variants in a separate series. > + val &= ~APP_LTSSM_ENABLE; > + writel(val, rcar->base + PCIERSTCTRL1); > + > + return 0; > + } > + > + val = dw_pcie_readl_dbi(dw, PCIE_PORT_FORCE); > + val |= PORT_FORCE_DO_DESKEW_FOR_SRIS; > + dw_pcie_writel_dbi(dw, PCIE_PORT_FORCE, val); > + > + val = readl(rcar->base + PCIEMSR0); > + val |= APP_SRIS_MODE; > + writel(val, rcar->base + PCIEMSR0); > + > + /* > + * The R-Car Gen4 documents don't describe the PHY registers' name. Gen4 datasheet? or reference manual? - Mani
Hi Manivannan, > From: Manivannan Sadhasivam, Sent: Saturday, June 8, 2024 5:25 PM > > On Mon, May 20, 2024 at 04:42:59PM +0900, Yoshihiro Shimoda wrote: > > Add support for r8a779g0 (R-Car V4H). > > > > This driver previously supported r8a779f0 (R-Car S4-8). PCIe features > > of both r8a779f0 and r8a779g0 are almost all the same. For example: > > - PCI Express Base Specification Revision 4.0 > > - Root complex mode and endpoint mode are supported > > However, r8a779g0 requires specific firmware downloading, to > > initialize the PHY. Otherwise, the PCIe controller cannot work. > > > > The attached firmware file "104_PCIe_fw_addr_data_ver1.05.txt" in > > the manual is a text file. But, Renesas is not able to distribute > > the firmware freely. So, we require converting the text file > > to a binary before the driver runs by using the following script: > > > > $ awk '/^\s*0x[0-9A-Fa-f]{4}\s+0x[0-9A-Fa-f]{4}/ \ > > { print substr($2,5,2) substr($2,3,2) }' \ > > 104_PCIe_fw_addr_data_ver1.05.txt | xxd -p -r > \ > > rcar_gen4_pcie.bin > > $ sha1sum rcar_gen4_pcie.bin > > 1d0bd4b189b4eb009f5d564b1f93a79112994945 rcar_gen4_pcie.bin > > > > Signed-off-by: Yoshihiro Shimoda <yoshihiro.shimoda.uh@renesas.com> > > --- > > drivers/pci/controller/dwc/pcie-rcar-gen4.c | 194 +++++++++++++++++++- > > 1 file changed, 193 insertions(+), 1 deletion(-) > > > > diff --git a/drivers/pci/controller/dwc/pcie-rcar-gen4.c b/drivers/pci/controller/dwc/pcie-rcar-gen4.c > > index bcbf0a52890d..f766a9739e15 100644 > > --- a/drivers/pci/controller/dwc/pcie-rcar-gen4.c > > +++ b/drivers/pci/controller/dwc/pcie-rcar-gen4.c > > @@ -5,8 +5,10 @@ > > */ > > > > #include <linux/delay.h> > > +#include <linux/firmware.h> > > #include <linux/interrupt.h> > > #include <linux/io.h> > > +#include <linux/iopoll.h> > > #include <linux/module.h> > > #include <linux/of.h> > > #include <linux/pci.h> > > @@ -20,9 +22,10 @@ > > /* Renesas-specific */ > > /* PCIe Mode Setting Register 0 */ > > #define PCIEMSR0 0x0000 > > -#define BIFUR_MOD_SET_ON BIT(0) > > +#define APP_SRIS_MODE BIT(6) > > #define DEVICE_TYPE_EP 0 > > #define DEVICE_TYPE_RC BIT(4) > > +#define BIFUR_MOD_SET_ON BIT(0) > > > > /* PCIe Interrupt Status 0 */ > > #define PCIEINTSTS0 0x0084 > > @@ -37,19 +40,36 @@ > > #define PCIEDMAINTSTSEN 0x0314 > > #define PCIEDMAINTSTSEN_INIT GENMASK(15, 0) > > > > +/* Port Logic Registers 89 */ > > +#define PRTLGC89 0x0b70 > > + > > +/* Port Logic Registers 90 */ > > +#define PRTLGC90 0x0b74 > > + > > /* PCIe Reset Control Register 1 */ > > #define PCIERSTCTRL1 0x0014 > > #define APP_HOLD_PHY_RST BIT(16) > > #define APP_LTSSM_ENABLE BIT(0) > > > > +/* PCIe Power Management Control */ > > +#define PCIEPWRMNGCTRL 0x0070 > > +#define APP_CLK_REQ_N BIT(11) > > +#define APP_CLK_PM_EN BIT(10) > > + > > #define RCAR_NUM_SPEED_CHANGE_RETRIES 10 > > #define RCAR_MAX_LINK_SPEED 4 > > > > #define RCAR_GEN4_PCIE_EP_FUNC_DBI_OFFSET 0x1000 > > #define RCAR_GEN4_PCIE_EP_FUNC_DBI2_OFFSET 0x800 > > > > +/* About the firmware, please refer to the commit log */ > > Which commit log? This is no useful info to the developers. Please add the > actual information about the firmware here (whatever you mentioned in the commit > message). I got it. > > +#define RCAR_GEN4_PCIE_FIRMWARE_NAME "rcar_gen4_pcie.bin" > > +#define RCAR_GEN4_PCIE_FIRMWARE_BASE_ADDR 0xc000 > > +MODULE_FIRMWARE(RCAR_GEN4_PCIE_FIRMWARE_NAME); > > + > > struct rcar_gen4_pcie; > > struct rcar_gen4_pcie_drvdata { > > + void (*additional_common_init)(struct rcar_gen4_pcie *rcar); > > int (*ltssm_control)(struct rcar_gen4_pcie *rcar, bool enable); > > enum dw_pcie_device_mode mode; > > }; > > @@ -57,6 +77,7 @@ struct rcar_gen4_pcie_drvdata { > > struct rcar_gen4_pcie { > > struct dw_pcie dw; > > void __iomem *base; > > + void __iomem *phy_base; > > struct platform_device *pdev; > > const struct rcar_gen4_pcie_drvdata *drvdata; > > }; > > @@ -180,6 +201,9 @@ static int rcar_gen4_pcie_common_init(struct rcar_gen4_pcie *rcar) > > if (ret) > > goto err_unprepare; > > > > + if (rcar->drvdata->additional_common_init) > > + rcar->drvdata->additional_common_init(rcar); > > + > > return 0; > > > > err_unprepare: > > @@ -221,6 +245,10 @@ static void rcar_gen4_pcie_unprepare(struct rcar_gen4_pcie *rcar) > > > > static int rcar_gen4_pcie_get_resources(struct rcar_gen4_pcie *rcar) > > { > > + rcar->phy_base = devm_platform_ioremap_resource_byname(rcar->pdev, "phy"); > > + if (IS_ERR(rcar->phy_base)) > > + return PTR_ERR(rcar->phy_base); > > + > > /* Renesas-specific registers */ > > rcar->base = devm_platform_ioremap_resource_byname(rcar->pdev, "app"); > > > > @@ -514,6 +542,166 @@ static int r8a779f0_pcie_ltssm_control(struct rcar_gen4_pcie *rcar, bool enable) > > return 0; > > } > > > > +static void rcar_gen4_pcie_additional_common_init(struct rcar_gen4_pcie *rcar) > > +{ > > + struct dw_pcie *dw = &rcar->dw; > > + u32 val; > > + > > + /* > > + * The SoC manual said the register setting is required. Otherwise, > > + * linkup failed. > > This also can be dropped. I got it. > > + */ > > + val = dw_pcie_readl_dbi(dw, PCIE_PORT_LANE_SKEW); > > + val &= ~PORT_LANE_SKEW_INSERT_MASK; > > + if (dw->num_lanes < 4) > > + val |= BIT(6); > > + dw_pcie_writel_dbi(dw, PCIE_PORT_LANE_SKEW, val); > > + > > + val = readl(rcar->base + PCIEPWRMNGCTRL); > > + val |= APP_CLK_REQ_N | APP_CLK_PM_EN; > > + writel(val, rcar->base + PCIEPWRMNGCTRL); > > +} > > + > > +static void rcar_gen4_pcie_phy_reg_update_bits(struct rcar_gen4_pcie *rcar, > > + u32 offset, u32 mask, u32 val) > > +{ > > + u32 tmp; > > + > > + tmp = readl(rcar->phy_base + offset); > > + tmp &= ~mask; > > + tmp |= val; > > Use FIELD_* macros to avoid using the shift value. According to the bitfield.h, --- * FIELD_{GET,PREP} macros take as first parameter shifted mask * from which they extract the base mask and shift amount. * Mask must be a compilation time constant. --- So, since the mask is a variable here, we cannot use FIELD_* macros for this function. > > + writel(tmp, rcar->phy_base + offset); > > +} > > + > > +static int rcar_gen4_pcie_reg_test_bit(struct rcar_gen4_pcie *rcar, > > + u32 offset, u32 mask) > > +{ > > + struct dw_pcie *dw = &rcar->dw; > > + > > + if (dw_pcie_readl_dbi(dw, offset) & mask) > > + return -EAGAIN; > > Here a comment like below could be added at the top of the function > definition. > > /* > * SoC reference manual suggests checking port logic register bits > * during firmware write. If read returns non-zero value, then this > * function returns -EAGAIN indicating that the write needs to be done > * again. If read returns zero, then return 0 to indicate success. > */ I got it. > > + > > + return 0; > > +} > > + > > +static int rcar_gen4_pcie_download_phy_firmware(struct rcar_gen4_pcie *rcar) > > +{ > > + /* The check_addr values are magical numbers in the datasheet */ > > + const u32 check_addr[] = { 0x00101018, 0x00101118, 0x00101021, 0x00101121}; > > + struct dw_pcie *dw = &rcar->dw; > > + const struct firmware *fw; > > + unsigned int i, timeout; > > + u32 data; > > + int ret; > > + > > + ret = request_firmware(&fw, RCAR_GEN4_PCIE_FIRMWARE_NAME, dw->dev); > > + if (ret) { > > + dev_err(dw->dev, "%s: Requesting firmware failed (%s)\n", > > + __func__, RCAR_GEN4_PCIE_FIRMWARE_NAME); > > dev_err(dw->dev, "Failed to load firmware (%s): %d", > RCAR_GEN4_PCIE_FIRMWARE_NAME, ret); > > > + return ret; > > + } > > + > > + for (i = 0; i < (fw->size / 2); i++) { > > + data = fw->data[(i * 2) + 1] << 8 | fw->data[i * 2]; > > + timeout = 100; > > + do { > > + dw_pcie_writel_dbi(dw, PRTLGC89, RCAR_GEN4_PCIE_FIRMWARE_BASE_ADDR + i); > > + dw_pcie_writel_dbi(dw, PRTLGC90, data); > > + if (rcar_gen4_pcie_reg_test_bit(rcar, PRTLGC89, BIT(30)) >= 0) > > Can the return value > 0? No. So, it seems using "!" like below is better, I think. + if (!rcar_gen4_pcie_reg_test_bit(rcar, PRTLGC89, BIT(30))) > > + break; > > + if (!(--timeout)) { > > + ret = -ETIMEDOUT; > > + goto exit; > > + } > > + usleep_range(100, 200); > > + } while (1); > > + } > > + > > + rcar_gen4_pcie_phy_reg_update_bits(rcar, 0x0f8, BIT(17), BIT(17)); > > + > > + for (i = 0; i < ARRAY_SIZE(check_addr); i++) { > > + timeout = 100; > > + do { > > + dw_pcie_writel_dbi(dw, PRTLGC89, check_addr[i]); > > + ret = rcar_gen4_pcie_reg_test_bit(rcar, PRTLGC89, BIT(30)); > > + ret |= rcar_gen4_pcie_reg_test_bit(rcar, PRTLGC90, BIT(0)); > > + if (ret >= 0) > > Same as above. I got it. > > + break; > > + if (!(--timeout)) { > > + ret = -ETIMEDOUT; > > + goto exit; > > + } > > + usleep_range(100, 200); > > + } while (1); > > + } > > + > > + ret = 0; > > At this point, ret should be 0, right? It's right. So, I'll drop this. > > +exit: > > + release_firmware(fw); > > + > > + return ret; > > +} > > + > > +static int rcar_gen4_pcie_ltssm_control(struct rcar_gen4_pcie *rcar, bool enable) > > +{ > > + struct dw_pcie *dw = &rcar->dw; > > + u32 val; > > + int ret; > > + > > + if (!enable) { > > + val = readl(rcar->base + PCIERSTCTRL1); > > There is no need to use 'readl/writel' here (which has the barrier), but since > this driver is already using them, I'm not asking to change it now. But please > consider switching to _relaxed variants in a separate series. I got it. > > + val &= ~APP_LTSSM_ENABLE; > > + writel(val, rcar->base + PCIERSTCTRL1); > > + > > + return 0; > > + } > > + > > + val = dw_pcie_readl_dbi(dw, PCIE_PORT_FORCE); > > + val |= PORT_FORCE_DO_DESKEW_FOR_SRIS; > > + dw_pcie_writel_dbi(dw, PCIE_PORT_FORCE, val); > > + > > + val = readl(rcar->base + PCIEMSR0); > > + val |= APP_SRIS_MODE; > > + writel(val, rcar->base + PCIEMSR0); > > + > > + /* > > + * The R-Car Gen4 documents don't describe the PHY registers' name. > > Gen4 datasheet? or reference manual? I'll write "Gen4 datasheet" instead. Best regards, Yoshihiro Shimoda > - Mani > > -- > மணிவண்ணன் சதாசிவம்
On Tue, Jun 11, 2024 at 11:21 AM Yoshihiro Shimoda <yoshihiro.shimoda.uh@renesas.com> wrote: > > From: Manivannan Sadhasivam, Sent: Saturday, June 8, 2024 5:25 PM > > On Mon, May 20, 2024 at 04:42:59PM +0900, Yoshihiro Shimoda wrote: > > > +static void rcar_gen4_pcie_phy_reg_update_bits(struct rcar_gen4_pcie *rcar, > > > + u32 offset, u32 mask, u32 val) > > > +{ > > > + u32 tmp; > > > + > > > + tmp = readl(rcar->phy_base + offset); > > > + tmp &= ~mask; > > > + tmp |= val; > > > > Use FIELD_* macros to avoid using the shift value. > > According to the bitfield.h, > --- > * FIELD_{GET,PREP} macros take as first parameter shifted mask > * from which they extract the base mask and shift amount. > * Mask must be a compilation time constant. > --- > So, since the mask is a variable here, we cannot use FIELD_* macros for this function. Indeed. I tried introducing non-constant field_{prep,get}() helpers[1] in series [2], but there were some pushbacks. Feel free to up-vote ;-) [1] "[PATCH 01/17] bitfield: Add non-constant field_{prep,get}() helpers" https://lore.kernel.org/all/3a54a6703879d10f08cf0275a2a69297ebd2b1d4.1637592133.git.geert+renesas@glider.be/ [2] "[PATCH 00/17] Non-const bitfield helper conversions" https://lore.kernel.org/all/cover.1637592133.git.geert+renesas@glider.be/ Gr{oetje,eeting}s, Geert
On Tue, Jun 11, 2024 at 12:10:22PM +0200, Geert Uytterhoeven wrote: > On Tue, Jun 11, 2024 at 11:21 AM Yoshihiro Shimoda > <yoshihiro.shimoda.uh@renesas.com> wrote: > > > From: Manivannan Sadhasivam, Sent: Saturday, June 8, 2024 5:25 PM > > > On Mon, May 20, 2024 at 04:42:59PM +0900, Yoshihiro Shimoda wrote: > > > > +static void rcar_gen4_pcie_phy_reg_update_bits(struct rcar_gen4_pcie *rcar, > > > > + u32 offset, u32 mask, u32 val) > > > > +{ > > > > + u32 tmp; > > > > + > > > > + tmp = readl(rcar->phy_base + offset); > > > > + tmp &= ~mask; > > > > + tmp |= val; > > > > > > Use FIELD_* macros to avoid using the shift value. > > > > According to the bitfield.h, > > --- > > * FIELD_{GET,PREP} macros take as first parameter shifted mask > > * from which they extract the base mask and shift amount. > > * Mask must be a compilation time constant. > > --- > > So, since the mask is a variable here, we cannot use FIELD_* macros for this function. > > Indeed. > I just can't keep the constant factor in mind for some reason. > I tried introducing non-constant field_{prep,get}() helpers[1] in series > [2], but there were some pushbacks. > > Feel free to up-vote ;-) > For sure! This will be very useful, thanks. - Mani > [1] "[PATCH 01/17] bitfield: Add non-constant field_{prep,get}() helpers" > https://lore.kernel.org/all/3a54a6703879d10f08cf0275a2a69297ebd2b1d4.1637592133.git.geert+renesas@glider.be/ > > [2] "[PATCH 00/17] Non-const bitfield helper conversions" > https://lore.kernel.org/all/cover.1637592133.git.geert+renesas@glider.be/ > > Gr{oetje,eeting}s, > > Geert > > -- > Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org > > In personal conversations with technical people, I call myself a hacker. But > when I'm talking to journalists I just say "programmer" or something like that. > -- Linus Torvalds >
diff --git a/drivers/pci/controller/dwc/pcie-rcar-gen4.c b/drivers/pci/controller/dwc/pcie-rcar-gen4.c index bcbf0a52890d..f766a9739e15 100644 --- a/drivers/pci/controller/dwc/pcie-rcar-gen4.c +++ b/drivers/pci/controller/dwc/pcie-rcar-gen4.c @@ -5,8 +5,10 @@ */ #include <linux/delay.h> +#include <linux/firmware.h> #include <linux/interrupt.h> #include <linux/io.h> +#include <linux/iopoll.h> #include <linux/module.h> #include <linux/of.h> #include <linux/pci.h> @@ -20,9 +22,10 @@ /* Renesas-specific */ /* PCIe Mode Setting Register 0 */ #define PCIEMSR0 0x0000 -#define BIFUR_MOD_SET_ON BIT(0) +#define APP_SRIS_MODE BIT(6) #define DEVICE_TYPE_EP 0 #define DEVICE_TYPE_RC BIT(4) +#define BIFUR_MOD_SET_ON BIT(0) /* PCIe Interrupt Status 0 */ #define PCIEINTSTS0 0x0084 @@ -37,19 +40,36 @@ #define PCIEDMAINTSTSEN 0x0314 #define PCIEDMAINTSTSEN_INIT GENMASK(15, 0) +/* Port Logic Registers 89 */ +#define PRTLGC89 0x0b70 + +/* Port Logic Registers 90 */ +#define PRTLGC90 0x0b74 + /* PCIe Reset Control Register 1 */ #define PCIERSTCTRL1 0x0014 #define APP_HOLD_PHY_RST BIT(16) #define APP_LTSSM_ENABLE BIT(0) +/* PCIe Power Management Control */ +#define PCIEPWRMNGCTRL 0x0070 +#define APP_CLK_REQ_N BIT(11) +#define APP_CLK_PM_EN BIT(10) + #define RCAR_NUM_SPEED_CHANGE_RETRIES 10 #define RCAR_MAX_LINK_SPEED 4 #define RCAR_GEN4_PCIE_EP_FUNC_DBI_OFFSET 0x1000 #define RCAR_GEN4_PCIE_EP_FUNC_DBI2_OFFSET 0x800 +/* About the firmware, please refer to the commit log */ +#define RCAR_GEN4_PCIE_FIRMWARE_NAME "rcar_gen4_pcie.bin" +#define RCAR_GEN4_PCIE_FIRMWARE_BASE_ADDR 0xc000 +MODULE_FIRMWARE(RCAR_GEN4_PCIE_FIRMWARE_NAME); + struct rcar_gen4_pcie; struct rcar_gen4_pcie_drvdata { + void (*additional_common_init)(struct rcar_gen4_pcie *rcar); int (*ltssm_control)(struct rcar_gen4_pcie *rcar, bool enable); enum dw_pcie_device_mode mode; }; @@ -57,6 +77,7 @@ struct rcar_gen4_pcie_drvdata { struct rcar_gen4_pcie { struct dw_pcie dw; void __iomem *base; + void __iomem *phy_base; struct platform_device *pdev; const struct rcar_gen4_pcie_drvdata *drvdata; }; @@ -180,6 +201,9 @@ static int rcar_gen4_pcie_common_init(struct rcar_gen4_pcie *rcar) if (ret) goto err_unprepare; + if (rcar->drvdata->additional_common_init) + rcar->drvdata->additional_common_init(rcar); + return 0; err_unprepare: @@ -221,6 +245,10 @@ static void rcar_gen4_pcie_unprepare(struct rcar_gen4_pcie *rcar) static int rcar_gen4_pcie_get_resources(struct rcar_gen4_pcie *rcar) { + rcar->phy_base = devm_platform_ioremap_resource_byname(rcar->pdev, "phy"); + if (IS_ERR(rcar->phy_base)) + return PTR_ERR(rcar->phy_base); + /* Renesas-specific registers */ rcar->base = devm_platform_ioremap_resource_byname(rcar->pdev, "app"); @@ -514,6 +542,166 @@ static int r8a779f0_pcie_ltssm_control(struct rcar_gen4_pcie *rcar, bool enable) return 0; } +static void rcar_gen4_pcie_additional_common_init(struct rcar_gen4_pcie *rcar) +{ + struct dw_pcie *dw = &rcar->dw; + u32 val; + + /* + * The SoC manual said the register setting is required. Otherwise, + * linkup failed. + */ + val = dw_pcie_readl_dbi(dw, PCIE_PORT_LANE_SKEW); + val &= ~PORT_LANE_SKEW_INSERT_MASK; + if (dw->num_lanes < 4) + val |= BIT(6); + dw_pcie_writel_dbi(dw, PCIE_PORT_LANE_SKEW, val); + + val = readl(rcar->base + PCIEPWRMNGCTRL); + val |= APP_CLK_REQ_N | APP_CLK_PM_EN; + writel(val, rcar->base + PCIEPWRMNGCTRL); +} + +static void rcar_gen4_pcie_phy_reg_update_bits(struct rcar_gen4_pcie *rcar, + u32 offset, u32 mask, u32 val) +{ + u32 tmp; + + tmp = readl(rcar->phy_base + offset); + tmp &= ~mask; + tmp |= val; + writel(tmp, rcar->phy_base + offset); +} + +static int rcar_gen4_pcie_reg_test_bit(struct rcar_gen4_pcie *rcar, + u32 offset, u32 mask) +{ + struct dw_pcie *dw = &rcar->dw; + + if (dw_pcie_readl_dbi(dw, offset) & mask) + return -EAGAIN; + + return 0; +} + +static int rcar_gen4_pcie_download_phy_firmware(struct rcar_gen4_pcie *rcar) +{ + /* The check_addr values are magical numbers in the datasheet */ + const u32 check_addr[] = { 0x00101018, 0x00101118, 0x00101021, 0x00101121}; + struct dw_pcie *dw = &rcar->dw; + const struct firmware *fw; + unsigned int i, timeout; + u32 data; + int ret; + + ret = request_firmware(&fw, RCAR_GEN4_PCIE_FIRMWARE_NAME, dw->dev); + if (ret) { + dev_err(dw->dev, "%s: Requesting firmware failed (%s)\n", + __func__, RCAR_GEN4_PCIE_FIRMWARE_NAME); + return ret; + } + + for (i = 0; i < (fw->size / 2); i++) { + data = fw->data[(i * 2) + 1] << 8 | fw->data[i * 2]; + timeout = 100; + do { + dw_pcie_writel_dbi(dw, PRTLGC89, RCAR_GEN4_PCIE_FIRMWARE_BASE_ADDR + i); + dw_pcie_writel_dbi(dw, PRTLGC90, data); + if (rcar_gen4_pcie_reg_test_bit(rcar, PRTLGC89, BIT(30)) >= 0) + break; + if (!(--timeout)) { + ret = -ETIMEDOUT; + goto exit; + } + usleep_range(100, 200); + } while (1); + } + + rcar_gen4_pcie_phy_reg_update_bits(rcar, 0x0f8, BIT(17), BIT(17)); + + for (i = 0; i < ARRAY_SIZE(check_addr); i++) { + timeout = 100; + do { + dw_pcie_writel_dbi(dw, PRTLGC89, check_addr[i]); + ret = rcar_gen4_pcie_reg_test_bit(rcar, PRTLGC89, BIT(30)); + ret |= rcar_gen4_pcie_reg_test_bit(rcar, PRTLGC90, BIT(0)); + if (ret >= 0) + break; + if (!(--timeout)) { + ret = -ETIMEDOUT; + goto exit; + } + usleep_range(100, 200); + } while (1); + } + + ret = 0; +exit: + release_firmware(fw); + + return ret; +} + +static int rcar_gen4_pcie_ltssm_control(struct rcar_gen4_pcie *rcar, bool enable) +{ + struct dw_pcie *dw = &rcar->dw; + u32 val; + int ret; + + if (!enable) { + val = readl(rcar->base + PCIERSTCTRL1); + val &= ~APP_LTSSM_ENABLE; + writel(val, rcar->base + PCIERSTCTRL1); + + return 0; + } + + val = dw_pcie_readl_dbi(dw, PCIE_PORT_FORCE); + val |= PORT_FORCE_DO_DESKEW_FOR_SRIS; + dw_pcie_writel_dbi(dw, PCIE_PORT_FORCE, val); + + val = readl(rcar->base + PCIEMSR0); + val |= APP_SRIS_MODE; + writel(val, rcar->base + PCIEMSR0); + + /* + * The R-Car Gen4 documents don't describe the PHY registers' name. + * But, the initialization procedure describes these offsets. So, + * this driver has magical offset numbers. + */ + rcar_gen4_pcie_phy_reg_update_bits(rcar, 0x700, BIT(28), 0); + rcar_gen4_pcie_phy_reg_update_bits(rcar, 0x700, BIT(20), 0); + rcar_gen4_pcie_phy_reg_update_bits(rcar, 0x700, BIT(12), 0); + rcar_gen4_pcie_phy_reg_update_bits(rcar, 0x700, BIT(4), 0); + + rcar_gen4_pcie_phy_reg_update_bits(rcar, 0x148, GENMASK(23, 22), BIT(22)); + rcar_gen4_pcie_phy_reg_update_bits(rcar, 0x148, GENMASK(18, 16), GENMASK(17, 16)); + rcar_gen4_pcie_phy_reg_update_bits(rcar, 0x148, GENMASK(7, 6), BIT(6)); + rcar_gen4_pcie_phy_reg_update_bits(rcar, 0x148, GENMASK(2, 0), GENMASK(11, 0)); + rcar_gen4_pcie_phy_reg_update_bits(rcar, 0x1d4, GENMASK(16, 15), GENMASK(16, 15)); + rcar_gen4_pcie_phy_reg_update_bits(rcar, 0x514, BIT(26), BIT(26)); + rcar_gen4_pcie_phy_reg_update_bits(rcar, 0x0f8, BIT(16), 0); + rcar_gen4_pcie_phy_reg_update_bits(rcar, 0x0f8, BIT(19), BIT(19)); + + val = readl(rcar->base + PCIERSTCTRL1); + val &= ~APP_HOLD_PHY_RST; + writel(val, rcar->base + PCIERSTCTRL1); + + ret = readl_poll_timeout(rcar->phy_base + 0x0f8, val, !(val & BIT(18)), 100, 10000); + if (ret < 0) + return ret; + + ret = rcar_gen4_pcie_download_phy_firmware(rcar); + if (ret) + return ret; + + val = readl(rcar->base + PCIERSTCTRL1); + val |= APP_LTSSM_ENABLE; + writel(val, rcar->base + PCIERSTCTRL1); + + return 0; +} + static struct rcar_gen4_pcie_drvdata drvdata_r8a779f0_pcie = { .ltssm_control = r8a779f0_pcie_ltssm_control, .mode = DW_PCIE_RC_TYPE, @@ -525,10 +713,14 @@ static struct rcar_gen4_pcie_drvdata drvdata_r8a779f0_pcie_ep = { }; static struct rcar_gen4_pcie_drvdata drvdata_rcar_gen4_pcie = { + .additional_common_init = rcar_gen4_pcie_additional_common_init, + .ltssm_control = rcar_gen4_pcie_ltssm_control, .mode = DW_PCIE_RC_TYPE, }; static struct rcar_gen4_pcie_drvdata drvdata_rcar_gen4_pcie_ep = { + .additional_common_init = rcar_gen4_pcie_additional_common_init, + .ltssm_control = rcar_gen4_pcie_ltssm_control, .mode = DW_PCIE_EP_TYPE, };
Add support for r8a779g0 (R-Car V4H). This driver previously supported r8a779f0 (R-Car S4-8). PCIe features of both r8a779f0 and r8a779g0 are almost all the same. For example: - PCI Express Base Specification Revision 4.0 - Root complex mode and endpoint mode are supported However, r8a779g0 requires specific firmware downloading, to initialize the PHY. Otherwise, the PCIe controller cannot work. The attached firmware file "104_PCIe_fw_addr_data_ver1.05.txt" in the manual is a text file. But, Renesas is not able to distribute the firmware freely. So, we require converting the text file to a binary before the driver runs by using the following script: $ awk '/^\s*0x[0-9A-Fa-f]{4}\s+0x[0-9A-Fa-f]{4}/ \ { print substr($2,5,2) substr($2,3,2) }' \ 104_PCIe_fw_addr_data_ver1.05.txt | xxd -p -r > \ rcar_gen4_pcie.bin $ sha1sum rcar_gen4_pcie.bin 1d0bd4b189b4eb009f5d564b1f93a79112994945 rcar_gen4_pcie.bin Signed-off-by: Yoshihiro Shimoda <yoshihiro.shimoda.uh@renesas.com> --- drivers/pci/controller/dwc/pcie-rcar-gen4.c | 194 +++++++++++++++++++- 1 file changed, 193 insertions(+), 1 deletion(-)