On Thu, Oct 1, 2020 at 8:47 AM Lorenzo Pieralisi
<lorenzo.pieralisi(a)arm.com> wrote:
On Wed, Sep 30, 2020 at 04:50:05PM -0400, Jim Quinlan wrote:
> On Wed, Sep 30, 2020 at 2:12 AM Dan Carpenter <dan.carpenter(a)oracle.com>
wrote:
> >
> > Hi Jim,
> >
> > First bad commit (maybe != root cause):
> >
> > tree:
https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git
master
> > head: 49e7e3e905e437a02782019570f70997e2da9101
> > commit: 5d98ac4e2823dcfd5e8a2ac3c71ec1ed6cdd1f54 [9468/11956] PCI: brcmstb: Set
additional internal memory DMA viewport sizes
> > config: arm64-randconfig-m031-20200929 (attached as .config)
> > compiler: aarch64-linux-gcc (GCC) 9.3.0
> >
> > If you fix the issue, kindly add following tag as appropriate
> > Reported-by: kernel test robot <lkp(a)intel.com>
> > Reported-by: Dan Carpenter <dan.carpenter(a)oracle.com>
>
> Hi Dan & Lorenzo,
>
> I have a concern: will my PCIe commits eventually be merged with
> Christoph's and myself's DMA commits (see
>
https://lkml.org/lkml/2020/9/16/80, my commit is 6/6), as the former
> commits depend on the latter?
What do you mean by "merged" ? If what you are saying is that there
are dependencies (build or functional) we may have to coordinate the
PCI PR and Christoph's.
Hi Lorenzo,
Originally the BrcmSTB PCIe PR contained a commit that changed how
dma-ranges were collected and stored. Since this commit affected many
files as well as the DMA subsystem and took on a life of its own, we
broke it off from the other PCIe commits. Christoph also added a
number of other DMA commits to my sole DMA commit.
I do not believe that there are any build dependencies between the
PCIe PR and the DMA PR. I believe that the RaspberryPi will work
with the current PCIe PR even if the DMA PR is not present. But the
BrcmSTB driver will not function for any non RaspberryPi Broadcom STB
chips without the DMA PR.
Of course this PCIe driver did not previously work for these Broadcom
STB chips, but now, w/o the DMA PR, the PCIe probe() will be called
and the driver will fail in ways I haven't tested.
I apologize for not making this clear sooner but I assumed you were
aware of the dependence.
Regards,
Jim
Lorenzo
> Regardless, I will send a fix soon.
>
> Thanks,
> Jim
>
> >
> > smatch warnings:
> > drivers/pci/controller/pcie-brcmstb.c:1270 brcm_pcie_probe() warn:
'pcie->clk' not released on lines: 1237.
> >
> > vim +1270 drivers/pci/controller/pcie-brcmstb.c
> >
> > c0452137034bda8 Jim Quinlan 2019-12-16 1176 static int
brcm_pcie_probe(struct platform_device *pdev)
> > c0452137034bda8 Jim Quinlan 2019-12-16 1177 {
> > 40ca1bf580ef24d Jim Quinlan 2019-12-16 1178 struct
device_node *np = pdev->dev.of_node, *msi_np;
> > c0452137034bda8 Jim Quinlan 2019-12-16 1179 struct
pci_host_bridge *bridge;
> > 44331189f9082c7 Nicolas Saenz Julienne 2020-05-05 1180 struct
device_node *fw_np;
> > 1cf1b0a6dd95250 Jim Quinlan 2020-09-11 1181 const struct
pcie_cfg_data *data;
> > c0452137034bda8 Jim Quinlan 2019-12-16 1182 struct
brcm_pcie *pcie;
> > c0452137034bda8 Jim Quinlan 2019-12-16 1183 int ret;
> > c0452137034bda8 Jim Quinlan 2019-12-16 1184
> > 44331189f9082c7 Nicolas Saenz Julienne 2020-05-05 1185 /*
> > 44331189f9082c7 Nicolas Saenz Julienne 2020-05-05 1186 * We have to
wait for Raspberry Pi's firmware interface to be up as a
> > 44331189f9082c7 Nicolas Saenz Julienne 2020-05-05 1187 * PCI fixup,
rpi_firmware_init_vl805(), depends on it. This driver's
> > 44331189f9082c7 Nicolas Saenz Julienne 2020-05-05 1188 * probe can
race with the firmware interface's (see
> > 44331189f9082c7 Nicolas Saenz Julienne 2020-05-05 1189 *
drivers/firmware/raspberrypi.c) and potentially break the PCI fixup.
> > 44331189f9082c7 Nicolas Saenz Julienne 2020-05-05 1190 */
> > 44331189f9082c7 Nicolas Saenz Julienne 2020-05-05 1191 fw_np =
of_find_compatible_node(NULL, NULL,
> > 44331189f9082c7 Nicolas Saenz Julienne 2020-05-05 1192
"raspberrypi,bcm2835-firmware");
> > 44331189f9082c7 Nicolas Saenz Julienne 2020-05-05 1193 if (fw_np
&& !rpi_firmware_get(fw_np)) {
> > 44331189f9082c7 Nicolas Saenz Julienne 2020-05-05 1194
of_node_put(fw_np);
> > 44331189f9082c7 Nicolas Saenz Julienne 2020-05-05 1195 return
-EPROBE_DEFER;
> > 44331189f9082c7 Nicolas Saenz Julienne 2020-05-05 1196 }
> > 44331189f9082c7 Nicolas Saenz Julienne 2020-05-05 1197
of_node_put(fw_np);
> > 44331189f9082c7 Nicolas Saenz Julienne 2020-05-05 1198
> > c0452137034bda8 Jim Quinlan 2019-12-16 1199 bridge =
devm_pci_alloc_host_bridge(&pdev->dev, sizeof(*pcie));
> > c0452137034bda8 Jim Quinlan 2019-12-16 1200 if (!bridge)
> > c0452137034bda8 Jim Quinlan 2019-12-16 1201 return
-ENOMEM;
> > c0452137034bda8 Jim Quinlan 2019-12-16 1202
> > 1cf1b0a6dd95250 Jim Quinlan 2020-09-11 1203 data =
of_device_get_match_data(&pdev->dev);
> > 1cf1b0a6dd95250 Jim Quinlan 2020-09-11 1204 if (!data) {
> > 1cf1b0a6dd95250 Jim Quinlan 2020-09-11 1205
pr_err("failed to look up compatible string\n");
> > 1cf1b0a6dd95250 Jim Quinlan 2020-09-11 1206 return
-EINVAL;
> > 1cf1b0a6dd95250 Jim Quinlan 2020-09-11 1207 }
> > 1cf1b0a6dd95250 Jim Quinlan 2020-09-11 1208
> > c0452137034bda8 Jim Quinlan 2019-12-16 1209 pcie =
pci_host_bridge_priv(bridge);
> > c0452137034bda8 Jim Quinlan 2019-12-16 1210 pcie->dev =
&pdev->dev;
> > c0452137034bda8 Jim Quinlan 2019-12-16 1211 pcie->np =
np;
> > 1cf1b0a6dd95250 Jim Quinlan 2020-09-11 1212
pcie->reg_offsets = data->offsets;
> > 1cf1b0a6dd95250 Jim Quinlan 2020-09-11 1213 pcie->type =
data->type;
> > 04356ac30771091 Jim Quinlan 2020-09-11 1214
pcie->perst_set = data->perst_set;
> > 04356ac30771091 Jim Quinlan 2020-09-11 1215
pcie->bridge_sw_init_set = data->bridge_sw_init_set;
> > c0452137034bda8 Jim Quinlan 2019-12-16 1216
> > 3cf0eead9fb895d Dejin Zheng 2020-07-08 1217 pcie->base =
devm_platform_ioremap_resource(pdev, 0);
> > c0452137034bda8 Jim Quinlan 2019-12-16 1218 if
(IS_ERR(pcie->base))
> > c0452137034bda8 Jim Quinlan 2019-12-16 1219 return
PTR_ERR(pcie->base);
> > c0452137034bda8 Jim Quinlan 2019-12-16 1220
> > c0452137034bda8 Jim Quinlan 2019-12-16 1221 pcie->clk =
devm_clk_get_optional(&pdev->dev, "sw_pcie");
> > c0452137034bda8 Jim Quinlan 2019-12-16 1222 if
(IS_ERR(pcie->clk))
> > c0452137034bda8 Jim Quinlan 2019-12-16 1223 return
PTR_ERR(pcie->clk);
> > c0452137034bda8 Jim Quinlan 2019-12-16 1224
> > c0452137034bda8 Jim Quinlan 2019-12-16 1225 ret =
of_pci_get_max_link_speed(np);
> > c0452137034bda8 Jim Quinlan 2019-12-16 1226 pcie->gen =
(ret < 0) ? 0 : ret;
> > c0452137034bda8 Jim Quinlan 2019-12-16 1227
> > c0452137034bda8 Jim Quinlan 2019-12-16 1228 pcie->ssc =
of_property_read_bool(np, "brcm,enable-ssc");
> > c0452137034bda8 Jim Quinlan 2019-12-16 1229
> > c0452137034bda8 Jim Quinlan 2019-12-16 1230 ret =
clk_prepare_enable(pcie->clk);
> > c0452137034bda8 Jim Quinlan 2019-12-16 1231 if (ret) {
> > c0452137034bda8 Jim Quinlan 2019-12-16 1232
dev_err(&pdev->dev, "could not enable clock\n");
> > c0452137034bda8 Jim Quinlan 2019-12-16 1233 return
ret;
> > c0452137034bda8 Jim Quinlan 2019-12-16 1234 }
> > b98f52bc6495110 Jim Quinlan 2020-09-11 1235 pcie->rescal
= devm_reset_control_get_optional_shared(&pdev->dev, "rescal");
> > b98f52bc6495110 Jim Quinlan 2020-09-11 1236 if
(IS_ERR(pcie->rescal))
> > b98f52bc6495110 Jim Quinlan 2020-09-11 1237 return
PTR_ERR(pcie->rescal);
> >
^^^^^^^^^^^^^^^^^^^^^^^^^^^^
> > Smatch complains that some of these don't do a "goto fail;"
should do
> > the clk_disable_unprepare(pcie->clk); from __brcm_pcie_remove(). The
> > clk is a devm_ clock. Do devm_ clocks need to be unprepared?
> >
> > b98f52bc6495110 Jim Quinlan 2020-09-11 1238
> > b98f52bc6495110 Jim Quinlan 2020-09-11 1239 ret =
reset_control_deassert(pcie->rescal);
> > b98f52bc6495110 Jim Quinlan 2020-09-11 1240 if (ret)
> > b98f52bc6495110 Jim Quinlan 2020-09-11 1241
dev_err(&pdev->dev, "failed to deassert 'rescal'\n");
> > b98f52bc6495110 Jim Quinlan 2020-09-11 1242
> > b98f52bc6495110 Jim Quinlan 2020-09-11 1243 ret =
brcm_phy_start(pcie);
> > b98f52bc6495110 Jim Quinlan 2020-09-11 1244 if (ret) {
> > b98f52bc6495110 Jim Quinlan 2020-09-11 1245
reset_control_assert(pcie->rescal);
> > b98f52bc6495110 Jim Quinlan 2020-09-11 1246 return
ret;
> > b98f52bc6495110 Jim Quinlan 2020-09-11 1247 }
> > c0452137034bda8 Jim Quinlan 2019-12-16 1248
> > c0452137034bda8 Jim Quinlan 2019-12-16 1249 ret =
brcm_pcie_setup(pcie);
> > c0452137034bda8 Jim Quinlan 2019-12-16 1250 if (ret)
> > c0452137034bda8 Jim Quinlan 2019-12-16 1251 goto
fail;
> > c0452137034bda8 Jim Quinlan 2019-12-16 1252
> > 40ca1bf580ef24d Jim Quinlan 2019-12-16 1253 msi_np =
of_parse_phandle(pcie->np, "msi-parent", 0);
> > 40ca1bf580ef24d Jim Quinlan 2019-12-16 1254 if
(pci_msi_enabled() && msi_np == pcie->np) {
> > 40ca1bf580ef24d Jim Quinlan 2019-12-16 1255 ret =
brcm_pcie_enable_msi(pcie);
> > 40ca1bf580ef24d Jim Quinlan 2019-12-16 1256 if
(ret) {
> > 40ca1bf580ef24d Jim Quinlan 2019-12-16 1257
dev_err(pcie->dev, "probe of internal MSI failed");
> > 40ca1bf580ef24d Jim Quinlan 2019-12-16 1258
goto fail;
> > 40ca1bf580ef24d Jim Quinlan 2019-12-16 1259 }
> > 40ca1bf580ef24d Jim Quinlan 2019-12-16 1260 }
> > 40ca1bf580ef24d Jim Quinlan 2019-12-16 1261
> > c0452137034bda8 Jim Quinlan 2019-12-16 1262 bridge->ops
= &brcm_pcie_ops;
> > c0452137034bda8 Jim Quinlan 2019-12-16 1263
bridge->sysdata = pcie;
> > c0452137034bda8 Jim Quinlan 2019-12-16 1264
> > c0452137034bda8 Jim Quinlan 2019-12-16 1265
platform_set_drvdata(pdev, pcie);
> > c0452137034bda8 Jim Quinlan 2019-12-16 1266
> > a37571fa9c04a9e Rob Herring 2020-05-22 1267 return
pci_host_probe(bridge);
> > c0452137034bda8 Jim Quinlan 2019-12-16 1268 fail:
> > c0452137034bda8 Jim Quinlan 2019-12-16 1269
__brcm_pcie_remove(pcie);
> > c0452137034bda8 Jim Quinlan 2019-12-16 @1270 return ret;
> > c0452137034bda8 Jim Quinlan 2019-12-16 1271 }
> >
> > ---
> > 0-DAY CI Kernel Test Service, Intel Corporation
> >
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org