Re: [PATCH RFC 1/2] phy: mvebu-cp110-utmi: add support for armada-380 utmi phys

Josua Mayer
2024-07-16T08:30:14Z

Am 15.07.24 um 20:05 schrieb Andrew Lunn:
>> @@ -191,8 +196,15 @@ static int mvebu_cp110_utmi_phy_power_on(struct phy *phy)
>> struct mvebu_cp110_utmi_port *port = phy_get_drvdata(phy);
>> struct mvebu_cp110_utmi *utmi = port->priv;
>> struct device *dev = &phy->dev;
>> + const void *match;
>> + enum mvebu_cp110_utmi_type type;
>> int ret;
>> u32 reg;
>> + u32 sel;
>> +
>> + match = of_device_get_match_data(dev);
Should be device_get_match_data?
>> + if (match)
>> + type = (enum mvebu_cp110_utmi_type)(uintptr_t)match;
>>
>> /* It is necessary to power off UTMI before configuration */
>> ret = mvebu_cp110_utmi_phy_power_off(phy);
>> @@ -208,16 +220,38 @@ static int mvebu_cp110_utmi_phy_power_on(struct phy *phy)
>> * to UTMI0 or to UTMI1 PHY port, but not to both.
>> */
>> if (port->dr_mode == USB_DR_MODE_PERIPHERAL) {
>> + switch (type) {
> Just looking at this, i'm surprised there is not a warning about
> type possibly being uninitialled.
Curious indeed. However I have not seen any compiler warnings
for uninitialized int (enum) recently.

I copied the pattern from drivers/gpu/drm/tiny/repaper.c,
there however is always an else case.

>
>> @@ -285,6 +320,8 @@ static int mvebu_cp110_utmi_phy_probe(struct platform_device *pdev)
>> struct mvebu_cp110_utmi *utmi;
>> struct phy_provider *provider;
>> struct device_node *child;
>> + const void *match;
>> + enum mvebu_cp110_utmi_type type;
>> u32 usb_devices = 0;
>>
>> utmi = devm_kzalloc(dev, sizeof(*utmi), GFP_KERNEL);
>> @@ -293,6 +330,10 @@ static int mvebu_cp110_utmi_phy_probe(struct platform_device *pdev)
>>
>> utmi->dev = dev;
>>
>> + match = of_device_get_match_data(dev);
>> + if (match)
>> + type = (enum mvebu_cp110_utmi_type)(uintptr_t)match;
>> +
>> /* Get system controller region */
>> utmi->syscon = syscon_regmap_lookup_by_phandle(dev->of_node,
>> "marvell,system-controller");
>> @@ -326,6 +367,18 @@ static int mvebu_cp110_utmi_phy_probe(struct platform_device *pdev)
>> return -ENOMEM;
>> }
>>
>> + /* Get port memory region */
>> + switch (type) {
> Same here.
>
> Andrew