In the existing SoundWire code, Master Devices are not explicitly represented - only SoundWire Slave Devices are exposed (the use of capital letters follows the SoundWire specification conventions). With the existing code, the bus is handled without using a proper device, and bus->dev typically points to a platform device. The right thing to do as discussed in multiple reviews is use a device for each bus. The sdw_master_device addition is done with minimal internal plumbing and not exposed externally. The existing API based on sdw_bus_master_add() and sdw_bus_master_delete() will deal with the sdw_master_device life cycle, which minimizes changes to existing drivers. Note that the Intel code will be modified in follow-up patches (no impact on any platform since the connection with ASoC is not supported upstream so far).
Unified · Split
| @@ -4,7 +4,7 @@ · 3 unmodified lines | |||
| 4 | # | 4 | # |
| 5 | 5 | ||
| 6 | #Bus Objs | 6 | #Bus Objs |
| 7 | soundwire-bus-objs := bus_type.o bus.o slave.o mipi_disco.o stream.o | 7 | soundwire-bus-objs := bus_type.o bus.o master.o slave.o mipi_disco.o stream.o |
| 8 | obj-$(CONFIG_SOUNDWIRE) += soundwire-bus.o | 8 | obj-$(CONFIG_SOUNDWIRE) += soundwire-bus.o |
| 9 | 9 | ||
| 10 | ifdef CONFIG_DEBUG_FS | 10 | ifdef CONFIG_DEBUG_FS |
| @@ -37,14 +37,21 @@ int sdw_bus_master_add(struct sdw_bus *bus, struct device *parent, · 36 unmodified lines | |||
| 37 | struct sdw_master_prop *prop = NULL; | 37 | struct sdw_master_prop *prop = NULL; |
| 38 | int ret; | 38 | int ret; |
| 39 | 39 | ||
| 40 | if (!bus->dev) { | 40 | if (!parent) { |
| 41 | pr_err("SoundWire bus has no device\n"); | 41 | pr_err("SoundWire parent device is not set\n"); |
| 42 | return -ENODEV; | 42 | return -ENODEV; |
| 43 | } | 43 | } |
| 44 | 44 | ||
| 45 | ret = sdw_get_id(bus); | 45 | ret = sdw_get_id(bus); |
| 46 | if (ret) { | 46 | if (ret) { |
| 47 | dev_err(bus->dev, "Failed to get bus id\n"); | 47 | dev_err(parent, "Failed to get bus id\n"); |
| 48 | return ret; | ||
| 49 | } | ||
| 50 | |||
| 51 | ret = sdw_master_device_add(bus, parent, fwnode); | ||
| 52 | if (ret) { | ||
| 53 | dev_err(parent, "Failed to add master device at link %d\n", | ||
| 54 | bus->link_id); | ||
| 48 | return ret; | 55 | return ret; |
| 49 | } | 56 | } |
| 50 | 57 | ||
| @@ -161,6 +168,7 @@ static int sdw_delete_slave(struct device *dev, void *data) · 110 unmodified lines | |||
| 161 | void sdw_bus_master_delete(struct sdw_bus *bus) | 168 | void sdw_bus_master_delete(struct sdw_bus *bus) |
| 162 | { | 169 | { |
| 163 | device_for_each_child(bus->dev, NULL, sdw_delete_slave); | 170 | device_for_each_child(bus->dev, NULL, sdw_delete_slave); |
| 171 | sdw_master_device_del(bus); | ||
| 164 | 172 | ||
| 165 | sdw_bus_debugfs_exit(bus); | 173 | sdw_bus_debugfs_exit(bus); |
| 166 | ida_free(&sdw_ida, bus->id); | 174 | ida_free(&sdw_ida, bus->id); |
| @@ -19,6 +19,9 @@ static inline int sdw_acpi_find_slaves(struct sdw_bus *bus) · 18 unmodified lines | |||
| 19 | int sdw_of_find_slaves(struct sdw_bus *bus); | 19 | int sdw_of_find_slaves(struct sdw_bus *bus); |
| 20 | void sdw_extract_slave_id(struct sdw_bus *bus, | 20 | void sdw_extract_slave_id(struct sdw_bus *bus, |
| 21 | u64 addr, struct sdw_slave_id *id); | 21 | u64 addr, struct sdw_slave_id *id); |
| 22 | int sdw_master_device_add(struct sdw_bus *bus, struct device *parent, | ||
| 23 | struct fwnode_handle *fwnode); | ||
| 24 | int sdw_master_device_del(struct sdw_bus *bus); | ||
| 22 | 25 | ||
| 23 | #ifdef CONFIG_DEBUG_FS | 26 | #ifdef CONFIG_DEBUG_FS |
| 24 | void sdw_bus_debugfs_init(struct sdw_bus *bus); | 27 | void sdw_bus_debugfs_init(struct sdw_bus *bus); |
| @@ -1099,7 +1099,6 @@ static int intel_probe(struct platform_device *pdev) · 1098 unmodified lines | |||
| 1099 | sdw->cdns.registers = sdw->link_res->registers; | 1099 | sdw->cdns.registers = sdw->link_res->registers; |
| 1100 | sdw->cdns.instance = sdw->instance; | 1100 | sdw->cdns.instance = sdw->instance; |
| 1101 | sdw->cdns.msg_count = 0; | 1101 | sdw->cdns.msg_count = 0; |
| 1102 | sdw->cdns.bus.dev = &pdev->dev; | ||
| 1103 | sdw->cdns.bus.link_id = pdev->id; | 1102 | sdw->cdns.bus.link_id = pdev->id; |
| 1104 | 1103 | ||
| 1105 | sdw_cdns_probe(&sdw->cdns); | 1104 | sdw_cdns_probe(&sdw->cdns); |
| @@ -0,0 +1,81 @@ | |||
| 1 | // SPDX-License-Identifier: GPL-2.0-only | ||
| 2 | // Copyright(c) 2019-2020 Intel Corporation. | ||
| 3 | |||
| 4 | #include <linux/device.h> | ||
| 5 | #include <linux/acpi.h> | ||
| 6 | #include <linux/soundwire/sdw.h> | ||
| 7 | #include <linux/soundwire/sdw_type.h> | ||
| 8 | #include "bus.h" | ||
| 9 | |||
| 10 | static void sdw_master_device_release(struct device *dev) | ||
| 11 | { | ||
| 12 | struct sdw_master_device *md = dev_to_sdw_master_device(dev); | ||
| 13 | |||
| 14 | kfree(md); | ||
| 15 | } | ||
| 16 | |||
| 17 | struct device_type sdw_master_type = { | ||
| 18 | .name = "soundwire_master", | ||
| 19 | .release = sdw_master_device_release, | ||
| 20 | }; | ||
| 21 | |||
| 22 | /** | ||
| 23 | * sdw_master_device_add() - create a Linux Master Device representation. | ||
| 24 | * @bus: SDW bus instance | ||
| 25 | * @parent: parent device | ||
| 26 | * @fwnode: firmware node handle | ||
| 27 | */ | ||
| 28 | int sdw_master_device_add(struct sdw_bus *bus, struct device *parent, | ||
| 29 | struct fwnode_handle *fwnode) | ||
| 30 | { | ||
| 31 | struct sdw_master_device *md; | ||
| 32 | int ret; | ||
| 33 | |||
| 34 | if (!parent) | ||
| 35 | return -EINVAL; | ||
| 36 | |||
| 37 | md = kzalloc(sizeof(*md), GFP_KERNEL); | ||
| 38 | if (!md) | ||
| 39 | return -ENOMEM; | ||
| 40 | |||
| 41 | md->dev.bus = &sdw_bus_type; | ||
| 42 | md->dev.type = &sdw_master_type; | ||
| 43 | md->dev.parent = parent; | ||
| 44 | md->dev.of_node = parent->of_node; | ||
| 45 | md->dev.fwnode = fwnode; | ||
| 46 | md->dev.dma_mask = parent->dma_mask; | ||
| 47 | |||
| 48 | dev_set_name(&md->dev, "sdw-master-%d", bus->id); | ||
| 49 | |||
| 50 | ret = device_register(&md->dev); | ||
| 51 | if (ret) { | ||
| 52 | dev_err(parent, "Failed to add master: ret %d\n", ret); | ||
| 53 | /* | ||
| 54 | * On err, don't free but drop ref as this will be freed | ||
| 55 | * when release method is invoked. | ||
| 56 | */ | ||
| 57 | put_device(&md->dev); | ||
| 58 | goto device_register_err; | ||
| 59 | } | ||
| 60 | |||
| 61 | /* add shortcuts to improve code readability/compactness */ | ||
| 62 | md->bus = bus; | ||
| 63 | bus->dev = &md->dev; | ||
| 64 | bus->md = md; | ||
| 65 | |||
| 66 | device_register_err: | ||
| 67 | return ret; | ||
| 68 | } | ||
| 69 | |||
| 70 | /** | ||
| 71 | * sdw_master_device_del() - delete a Linux Master Device representation. | ||
| 72 | * @bus: bus handle | ||
| 73 | * | ||
| 74 | * This function is the dual of sdw_master_device_add() | ||
| 75 | */ | ||
| 76 | int sdw_master_device_del(struct sdw_bus *bus) | ||
| 77 | { | ||
| 78 | device_unregister(bus->dev); | ||
| 79 | |||
| 80 | return 0; | ||
| 81 | } | ||
| @@ -784,7 +784,6 @@ static int qcom_swrm_probe(struct platform_device *pdev) · 783 unmodified lines | |||
| 784 | mutex_init(&ctrl->port_lock); | 784 | mutex_init(&ctrl->port_lock); |
| 785 | INIT_WORK(&ctrl->slave_work, qcom_swrm_slave_wq); | 785 | INIT_WORK(&ctrl->slave_work, qcom_swrm_slave_wq); |
| 786 | 786 | ||
| 787 | ctrl->bus.dev = dev; | ||
| 788 | ctrl->bus.ops = &qcom_swrm_ops; | 787 | ctrl->bus.ops = &qcom_swrm_ops; |
| 789 | ctrl->bus.port_ops = &qcom_swrm_port_ops; | 788 | ctrl->bus.port_ops = &qcom_swrm_port_ops; |
| 790 | ctrl->bus.compute_params = &qcom_swrm_compute_params; | 789 | ctrl->bus.compute_params = &qcom_swrm_compute_params; |
| @@ -632,6 +632,19 @@ struct sdw_slave { · 631 unmodified lines | |||
| 632 | 632 | ||
| 633 | #define dev_to_sdw_dev(_dev) container_of(_dev, struct sdw_slave, dev) | 633 | #define dev_to_sdw_dev(_dev) container_of(_dev, struct sdw_slave, dev) |
| 634 | 634 | ||
| 635 | /** | ||
| 636 | * struct sdw_master_device - SoundWire 'Master Device' representation | ||
| 637 | * @dev: Linux device for this Master | ||
| 638 | * @bus: Bus handle shortcut | ||
| 639 | */ | ||
| 640 | struct sdw_master_device { | ||
| 641 | struct device dev; | ||
| 642 | struct sdw_bus *bus; | ||
| 643 | }; | ||
| 644 | |||
| 645 | #define dev_to_sdw_master_device(d) \ | ||
| 646 | container_of(d, struct sdw_master_device, dev) | ||
| 647 | |||
| 635 | struct sdw_driver { | 648 | struct sdw_driver { |
| 636 | const char *name; | 649 | const char *name; |
| 637 | 650 | ||
| @@ -787,7 +800,8 @@ struct sdw_master_ops { · 149 unmodified lines | |||
| 787 | 800 | ||
| 788 | /** | 801 | /** |
| 789 | * struct sdw_bus - SoundWire bus | 802 | * struct sdw_bus - SoundWire bus |
| 790 | * @dev: Master linux device | 803 | * @dev: Shortcut to &bus->md->dev to avoid changing the entire code. |
| 804 | * @md: Master device | ||
| 791 | * @link_id: Link id number, can be 0 to N, unique for each Master | 805 | * @link_id: Link id number, can be 0 to N, unique for each Master |
| 792 | * @id: bus system-wide unique id | 806 | * @id: bus system-wide unique id |
| 793 | * @slaves: list of Slaves on this bus | 807 | * @slaves: list of Slaves on this bus |
| @@ -813,6 +827,7 @@ struct sdw_master_ops { · 19 unmodified lines | |||
| 813 | */ | 827 | */ |
| 814 | struct sdw_bus { | 828 | struct sdw_bus { |
| 815 | struct device *dev; | 829 | struct device *dev; |
| 830 | struct sdw_master_device *md; | ||
| 816 | unsigned int link_id; | 831 | unsigned int link_id; |
| 817 | int id; | 832 | int id; |
| 818 | struct list_head slaves; | 833 | struct list_head slaves; |