linux-kernel - Re: [PATCH v1 5/5] pci: keystone: add pcie driver based on designware core driver

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <6048322.x3M3sdDEWO@wuerfel>
Date:	Thu, 15 May 2014 18:23:05 +0200
From:	Arnd Bergmann <arnd@...db.de>
To:	linux-arm-kernel@...ts.infradead.org
Cc:	Murali Karicheri <m-karicheri2@...com>,
	linux-kernel@...r.kernel.org, linux-pci@...r.kernel.org,
	Grygorii Strashko <grygorii.strashko@...com>,
	Mohit Kumar <mohit.kumar@...com>,
	Jingoo Han <jg1.han@...sung.com>,
	Santosh Shilimkar <santosh.shilimkar@...com>,
	Bjorn Helgaas <bhelgaas@...gle.com>
Subject: Re: [PATCH v1 5/5] pci: keystone: add pcie driver based on designware core driver

On Thursday 15 May 2014 12:01:32 Murali Karicheri wrote:

> +static int
> +keystone_pcie_fault(unsigned long addr, unsigned int fsr,
> +		struct pt_regs *regs)
> +{
> +	unsigned long instr = *(unsigned long *) instruction_pointer(regs);
> +
> +	if ((instr & 0x0e100090) == 0x00100090) {
> +		int reg = (instr >> 12) & 15;
> +
> +		regs->uregs[reg] = -1;
> +		regs->ARM_pc += 4;
> +	}
> +
> +	return 0;
> +}

This needs to check in the PCI host registers what happened and do
some appropriate action. If the fault was not caused by PCIe, it
should not be ignored here but get passed on to the next handler.

> +static int __init ks_pcie_probe(struct platform_device *pdev)
> +{
> +	struct device_node *np = pdev->dev.of_node;
> +	struct device *dev = &pdev->dev;
> +	struct keystone_pcie *ks_pcie;
> +	void __iomem *devstat;
> +	struct pcie_port *pp;
> +	struct resource *res;
> +	struct phy *phy;
> +	int ret = 0;
> +	u32 val;
> +
> +	ks_pcie = devm_kzalloc(&pdev->dev, sizeof(*ks_pcie),
> +				GFP_KERNEL);
> +	if (!ks_pcie) {
> +		dev_err(dev, "no memory for keystone pcie\n");
> +		return -ENOMEM;
> +	}
> +
> +	/* check if serdes phy needs to be enabled */
> +	if (of_get_property(np, "ti,init-phy", NULL) != NULL) {
> +		phy = devm_phy_get(dev, "pcie-phy");
> +			if (IS_ERR(phy))
> +				return PTR_ERR(phy);
> +
> +		ret = phy_init(phy);
> +		if (ret < 0)
> +			return ret;
> +	}

I think you can just call devm_phy_get() unconditionally here and
get rid of the ti,init-phy property. If there is no phy, don't
initialize it.

> +
> +	ret = add_pcie_port(ks_pcie, pdev);
> +	if (ret < 0)
> +		goto fail_clk;
> +
> +	platform_set_drvdata(pdev, ks_pcie);

Set the platform data first, then add the port.

> +	dev_info(dev, "pcie rc probe success\n");

Remove this.

> +#ifdef CONFIG_PCI_KEYSTONE
> +/*
> + * The KeyStone PCIe controller has maximum read request size of 256 bytes.
> + */
> +static void quirk_limit_readrequest(struct pci_dev *dev)
> +{
> +	int readrq = pcie_get_readrq(dev);
> +
> +	if (readrq > 256)
> +		pcie_set_readrq(dev, 256);
> +}
> +DECLARE_PCI_FIXUP_FINAL(PCI_ANY_ID, PCI_ANY_ID, quirk_limit_readrequest);
> +#endif /* CONFIG_PCI_KEYSTONE */

This doesn't work: you can't just limit do this for all devices just based
on PCI_KEYSTONE being enabled, you have to check if you are actually using
this controller.

	Arnd
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/