Re: [PATCH 3/3] fpga: xilinx-selectmap: add new driver

On 29/01/2024 23:56, Charles Perry wrote:

Xilinx 7 series FPGA can be programmed using a slave parallel port named
the SelectMAP interface in the datasheet. This slave interface is
compatible with the i.MX6 EIM bus controller but other types of external
memory mapped parallel bus might work.

xilinx-selectmap currently only supports the x8 mode where data is loaded
at one byte per rising edge of the clock, with the MSb of each byte
presented to the D0 pin.

The following DT fragment shows a valid configuration on a custom i.MX6
board (pinctrl not shown for readability):

&weim {
    status = "okay";
    ranges = <0 0 0x08000000 0x04000000>;

    fpga_mgr: fpga_programmer@0,0 {
        compatible = "xlnx,fpga-slave-selectmap";
        reg = <0 0 0x4000000>;
        fsl,weim-cs-timing = <0x00070031 0x00000142
                              0x00020000 0x00000000
                              0x0c000645 0x00000000>;
        prog_b-gpios = <&gpio5 5 GPIO_ACTIVE_LOW>;
        init-b-gpios = <&gpio5 8 GPIO_ACTIVE_LOW>;
        done-gpios = <&gpio2 30 GPIO_ACTIVE_HIGH>;
        csi-b-gpios = <&gpio3 19 GPIO_ACTIVE_LOW>;
        rdwr-b-gpios = <&gpio3 10 GPIO_ACTIVE_LOW>;
    };
};

Drop that example. First, it is not correct. Second, a correct one in
bindings is enough.

Signed-off-by: Charles Perry <redacted>
---

...

+static int xilinx_selectmap_probe(struct platform_device *pdev)
+{
+	struct xilinx_selectmap_conf *conf;
+	struct resource *r;
+	void __iomem *base;
+
+	conf = devm_kzalloc(&pdev->dev, sizeof(*conf), GFP_KERNEL);
+	if (!conf)
+		return -ENOMEM;
+
+	base = devm_platform_get_and_ioremap_resource(pdev, 0, &r);
+	if (IS_ERR(base))
+		return dev_err_probe(&pdev->dev, PTR_ERR(base), "ioremap error\n");
+	conf->base = base;
+
+	/* CSI_B is active low */
+	conf->csi_b = devm_gpiod_get_optional(&pdev->dev, "csi-b", GPIOD_OUT_HIGH);
+	if (IS_ERR(conf->csi_b))
+		return dev_err_probe(&pdev->dev, PTR_ERR(conf->csi_b),
+				     "Failed to get CSI_B gpio\n");
+
+	/* RDWR_B is active low */
+	conf->rdwr_b = devm_gpiod_get_optional(&pdev->dev, "rdwr-b", GPIOD_OUT_HIGH);
+	if (IS_ERR(conf->rdwr_b))
+		return dev_err_probe(&pdev->dev, PTR_ERR(conf->rdwr_b),
+				     "Failed to get RDWR_B gpio\n");
+
+	return xilinx_core_probe(&conf->core, &pdev->dev,
+							xilinx_selectmap_write,
+							xilinx_selectmap_apply_padding);

Totally messed indentation. Please run scripts/checkpatch.pl and fix
reported warnings. Some warnings can be ignored, but the code here looks
like it needs a fix. Feel free to get in touch if the warning is not clear.

+}
+
+static const struct of_device_id xlnx_selectmap_of_match[] = {
+		{ .compatible = "xlnx,fpga-slave-selectmap", },
+		{}
+};
+MODULE_DEVICE_TABLE(of, xlnx_selectmap_of_match);
+
+static struct platform_driver xilinx_slave_selectmap_driver = {
+	.driver = {
+		.name = "xilinx-slave-selectmap",
+		.of_match_table = of_match_ptr(xlnx_selectmap_of_match),

Drop of_match_ptr, it leads to warnings.

+	},
+	.probe  = xilinx_selectmap_probe,
+};

Best regards,
Krzysztof