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]
Date: Tue, 26 Mar 2024 17:26:09 +0800
From: Haixu Cui <quic_haixcui@...cinc.com>
To: Viresh Kumar <viresh.kumar@...aro.org>,
        Harald Mommer
	<harald.mommer@...nsynergy.com>
CC: Mark Brown <broonie@...nel.org>, <linux-spi@...r.kernel.org>,
        <linux-kernel@...r.kernel.org>, <quic_ztu@...cinc.com>,
        Matti Moell
	<Matti.Moell@...nsynergy.com>,
        Mikhail Golubev
	<Mikhail.Golubev@...nsynergy.com>
Subject: Re: [PATCH v2 3/3] virtio-spi: Add virtio SPI driver.

Hi all,

On 3/20/2024 11:12 AM, Viresh Kumar wrote:
> Hi Harald,
> 
> On 19-03-24, 16:05, Harald Mommer wrote:
>> On 18.03.24 10:39, Haixu Cui wrote:
>>> On 3/4/2024 11:43 PM, Harald Mommer wrote:
>>>> +static int virtio_spi_probe(struct virtio_device *vdev)
>>>> +{
>>>> +    struct device_node *np = vdev->dev.parent->of_node;
>>>> +    struct virtio_spi_priv *priv;
>>>> +    struct spi_controller *ctrl;
>>>> +    int err;
>>>> +    u32 bus_num;
>>>> +    u16 csi;
>>>> +
>>>> +    ctrl = devm_spi_alloc_host(&vdev->dev, sizeof(*priv));
>>>> +    if (!ctrl)
>>>> +        return -ENOMEM;
>>>> +
>>>> +    priv = spi_controller_get_devdata(ctrl);
>>>> +    priv->vdev = vdev;
>>>> +    vdev->priv = priv;
>>>> +    dev_set_drvdata(&vdev->dev, ctrl);
>>>
>>>      ctrl->dev.of_node is not set, so the child node cannot be parsed. I
>>> would say, it's necessary to support the virtio spi device node in the
>>> format:
>>
>>
>> What you most probably want to have here is
>>
>>    ctrl->dev.of_node = np;
> 
> Looking at how i2c-virtio does it, it should be tied to the device itself
> instead of its parent:

Yes, it is
         ctrl->dev.of_node = vdev->dev.of_node;

> 
> ctrl->dev.of_node = vdev->dev.of_node;
> 
>>>      virtio-spi@...13000 {
>>>          compatible = "virtio,mmio";
>>>          reg = <0x4b013000 0x1000>;
>>>          interrupts = <0 497 4>;
>>>
>>>          spi {
>>>              compatible = "virtio,device2d";
>>>              #address-cells = <1>;
>>>              #size-cells = <0>;
>>>
>>>              spidev {
>>>                  compatible = "xxx";
>>>                  reg = <0>;
>>>                  spi-max-frequency = <100000>;
>>>              };
>>>          };
>>>      };
> 
> Right, some work was done in the past to standardize these compatibles:
> 
> $ git log -p --stat --reverse 0d8c9e7d4b40..694a1116b405
> 
     Here I would like the inner layer "spidev", to match then probe the 
spidev driver, the "reg" is the chip select index. "spi-max-frequency" 
is not necessary, while It doesn't matter.

     You can also customize the inner layer to match your own driver.

     In my test, I set the cs max number as 1, and in device tree node 
inner layer, reg as 0. So certainly spidev driver probed and spidev0.0 
is added successfully.

     But then the driver proceed to the following code, chip select 
index 0 device is created again, the driver fail with log: "chipselect 0 
already in use".

         for (csi = 0; csi < ctrl->num_chipselect; csi++) {
             dev_dbg(&vdev->dev, "Setting up CS=%u\n", csi);
             board_info.chip_select = csi;

             if (!(priv->mode_func_supported & VIRTIO_SPI_CS_HIGH))
                 board_info.mode = SPI_MODE_0;
             else
                 board_info.mode = SPI_MODE_0 | SPI_CS_HIGH;

             if (!spi_new_device(ctrl, &board_info)) {
                 dev_err(&vdev->dev, "Cannot setup device %u\n", csi);
                 spi_unregister_controller(ctrl);
                 err = -ENODEV;
                 goto err_return;
             }
         }


     I hope I made myself clear. Thank you, Harald and Kumar. Best regards.


Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ