Le 15/01/2025 à 23:46, Sasha Finkelstein via B4 Relay a écrit :
From: Sasha Finkelstein <fnkl.kernel-Re5JQEeQqe8AvxtiuMwx3w@xxxxxxxxxxxxxxxx>
Adds a driver for Apple touchscreens using the Z2 protocol.
...
+static int apple_z2_upload_firmware(struct apple_z2 *z2)
+{
+ const struct apple_z2_fw_hdr *fw_hdr;
+ size_t fw_idx = sizeof(struct apple_z2_fw_hdr);
+ int error;
+ u32 load_cmd;
+ u32 size;
+ u32 address;
+ char *data;
+ bool init;
+ size_t cal_size;
+
+ const struct firmware *fw __free(firmware) = NULL;
+ error = request_firmware(&fw, z2->fw_name, &z2->spidev->dev);
+ if (error) {
+ dev_err(&z2->spidev->dev, "unable to load firmware");
+ return error;
+ }
+
+ fw_hdr = (const struct apple_z2_fw_hdr *)fw->data;
+ if (le32_to_cpu(fw_hdr->magic) != APPLE_Z2_FW_MAGIC || le32_to_cpu(fw_hdr->version) != 1) {
+ dev_err(&z2->spidev->dev, "invalid firmware header");
+ return -EINVAL;
+ }
+
+ /*
+ * This will interrupt the upload half-way if the file is malformed
+ * As the device has no non-volatile storage to corrupt, and gets reset
+ * on boot anyway, this is fine.
+ */
+ while (fw_idx < fw->size) {
+ if (fw->size - fw_idx < 8) {
+ dev_err(&z2->spidev->dev, "firmware malformed");
+ return -EINVAL;
+ }
+
+ load_cmd = le32_to_cpu(*(__le32 *)(fw->data + fw_idx));
+ fw_idx += sizeof(u32);
+ if (load_cmd == LOAD_COMMAND_INIT_PAYLOAD || load_cmd == LOAD_COMMAND_SEND_BLOB) {
+ size = le32_to_cpu(*(__le32 *)(fw->data + fw_idx));
+ fw_idx += sizeof(u32);
+ if (fw->size - fw_idx < size) {
+ dev_err(&z2->spidev->dev, "firmware malformed");
+ return -EINVAL;
+ }
+ init = load_cmd == LOAD_COMMAND_INIT_PAYLOAD;
+ error = apple_z2_send_firmware_blob(z2, fw->data + fw_idx,
+ size, init);
+ if (error)
+ return error;
+ fw_idx += size;
+ } else if (load_cmd == LOAD_COMMAND_SEND_CALIBRATION) {
+ address = le32_to_cpu(*(u32 *)(fw->data + fw_idx));
+ fw_idx += sizeof(u32);
+ cal_size = device_property_count_u8(&z2->spidev->dev, CAL_PROP_NAME);
+ if (cal_size != 0) {
+ size = cal_size + sizeof(struct apple_z2_hbpp_blob_hdr) + 4;
+ data = kzalloc(size, GFP_KERNEL);
+ error = apple_z2_build_cal_blob(z2, address, cal_size, data);
+ if (!error)
+ error = apple_z2_send_firmware_blob(z2, data, size, 16);
+ kfree(data);
+ if (error)
+ return error;
+ }
+ } else {
+ dev_err(&z2->spidev->dev, "firmware malformed");
Missing \n.
+ return -EINVAL;
+ }
+ if (fw_idx % 4 != 0)
+ fw_idx += 4 - (fw_idx % 4);
+ }
+
+
+ z2->booted = true;
+ apple_z2_read_packet(z2);
+ return 0;
+}
...
+static int apple_z2_probe(struct spi_device *spi)
+{
+ struct device *dev = &spi->dev;
+ struct apple_z2 *z2;
+ int error;
+
+ z2 = devm_kzalloc(dev, sizeof(*z2), GFP_KERNEL);
+ if (!z2)
+ return -ENOMEM;
+
+ z2->tx_buf = devm_kzalloc(dev, sizeof(struct apple_z2_read_interrupt_cmd), GFP_KERNEL);
+ z2->rx_buf = devm_kzalloc(dev, 4096, GFP_KERNEL);
This will allocate 8192 bytes because of the way the allocator works.
It needs around 40 bytes for the devm stuff + 4096 requested. So
rounding rules will allocate 8192 bytes.
So either you could allocate "for free" much more space, or you could
allocate (and document...)
z2->rx_buf = devm_kzalloc(dev, 4096 - sizeof(struct devres), GFP_KERNEL);
or have an explicit devm_add_action_or_reset() that would require less
memory, but would add some LoC.
See
https://elixir.bootlin.com/linux/v6.13-rc3/source/drivers/base/devres.c#L97
+ if (!z2->tx_buf || !z2->rx_buf)
+ return -ENOMEM;
+
+ z2->spidev = spi;
+ init_completion(&z2->boot_irq);
+ spi_set_drvdata(spi, z2);
+
+ /* Reset the device on boot */
+ z2->reset_gpio = devm_gpiod_get(dev, "reset", GPIOD_OUT_HIGH);
+ if (IS_ERR(z2->reset_gpio))
+ return dev_err_probe(dev, PTR_ERR(z2->reset_gpio), "unable to get reset");
+
+ error = devm_request_threaded_irq(dev, z2->spidev->irq, NULL,
+ apple_z2_irq, IRQF_ONESHOT | IRQF_NO_AUTOEN,
+ "apple-z2-irq", spi);
+ if (error)
+ return dev_err_probe(dev, z2->spidev->irq, "unable to request irq");
Missing \n.
s/z2->spidev->irq/error/ ?
or maybe:
return dev_err_probe(dev, error, "unable to request irq %d\n",
z2->spidev->irq);
+
+ error = device_property_read_string(dev, "firmware-name", &z2->fw_name);
+ if (error)
+ return dev_err_probe(dev, error, "unable to get firmware name");
Missing \n.
+
+ z2->input_dev = devm_input_allocate_device(dev);
+ if (!z2->input_dev)
+ return -ENOMEM;
+ z2->input_dev->name = (char *)spi_get_device_id(spi)->driver_data;
+ z2->input_dev->phys = "apple_z2";
+ z2->input_dev->id.bustype = BUS_SPI;
+
+ /* Allocate the axes before setting from DT */
+ input_set_abs_params(z2->input_dev, ABS_MT_POSITION_X, 0, 0, 0, 0);
+ input_set_abs_params(z2->input_dev, ABS_MT_POSITION_Y, 0, 0, 0, 0);
+ touchscreen_parse_properties(z2->input_dev, true, &z2->props);
+ input_abs_set_res(z2->input_dev, ABS_MT_POSITION_X, 100);
+ input_abs_set_res(z2->input_dev, ABS_MT_POSITION_Y, 100);
+ input_set_abs_params(z2->input_dev, ABS_MT_WIDTH_MAJOR, 0, 65535, 0, 0);
+ input_set_abs_params(z2->input_dev, ABS_MT_WIDTH_MINOR, 0, 65535, 0, 0);
+ input_set_abs_params(z2->input_dev, ABS_MT_TOUCH_MAJOR, 0, 65535, 0, 0);
+ input_set_abs_params(z2->input_dev, ABS_MT_TOUCH_MINOR, 0, 65535, 0, 0);
+ input_set_abs_params(z2->input_dev, ABS_MT_ORIENTATION, -32768, 32767, 0, 0);
+
+ input_set_drvdata(z2->input_dev, z2);
Is it needed? (there is no input_get_drvdata())
+
+ error = input_mt_init_slots(z2->input_dev, 256, INPUT_MT_DIRECT);
+ if (error)
+ return dev_err_probe(dev, error, "unable to initialize multitouch slots");
Missing \n.
+
+ error = input_register_device(z2->input_dev);
+ if (error)
+ return dev_err_probe(dev, error, "unable to register input device");
Missing \n.
+
+ /* Wait for device reset to finish */
+ usleep_range(5000, 10000);
+ error = apple_z2_boot(z2);
+ if (error)
+ return error;
+ return 0;
Nitpick: These 4 lines could be just:
return apple_z2_boot(z2);
+}
...
+static DEFINE_SIMPLE_DEV_PM_OPS(apple_z2_pm, apple_z2_suspend, apple_z2_resume);
+
+static const struct of_device_id apple_z2_of_match[] = {
+ { .compatible = "apple,j293-touchbar" },
+ { .compatible = "apple,j493-touchbar" },
+ {},
Nitpick: Ending comma is not needed after a terminator.
+};
+MODULE_DEVICE_TABLE(of, apple_z2_of_match);
+
+static struct spi_device_id apple_z2_of_id[] = {
+ { .name = "j293-touchbar", .driver_data = (kernel_ulong_t)"MacBookPro17,1 Touch Bar" },
+ { .name = "j493-touchbar", .driver_data = (kernel_ulong_t)"Mac14,7 Touch Bar" },
+ {}
+};
+MODULE_DEVICE_TABLE(spi, apple_z2_of_id);
...
CJ
