RE: [PATCH v2 1/5] i3c: master: detach and free device if pre_assign_dyn_addr() fails

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hi Boris,

From: Boris Brezillon <boris.brezillon@xxxxxxxxxxxxx>
Date: Tue, Sep 03, 2019 at 11:52:37

> On Tue,  3 Sep 2019 12:35:50 +0200
> Vitor Soares <Vitor.Soares@xxxxxxxxxxxx> wrote:
> 
> > On pre_assing_dyn_addr() the devices that fail:
> >   i3c_master_setdasa_locked()
> >   i3c_master_reattach_i3c_dev()
> >   i3c_master_retrieve_dev_info()
> > 
> > are kept in memory and master->bus.devs list. This makes the i3c devices
> > without a dynamic address are sent on DEFSLVS CCC command. Fix this by
> > detaching and freeing the devices that fail on pre_assign_dyn_addr().
> > 
> > Signed-off-by: Vitor Soares <vitor.soares@xxxxxxxxxxxx>
> > ---
> > Changes in v2:
> >   - Move out detach/free the i3c_dev_desc from pre_assign_dyn_addr()
> 
> So, you decided to ignore my comment about leaving the i3c_dev_desc
> allocated and skipping entries that don't have a dynamic address when
> forging the DEFSLVS frame instead of doing this
> allocate/free/re-allocate dance, and more importantly, you didn't even
> bother explaining why.
> 
> I'm not willing to accept this patch unless you come up with solid
> reasons.

I think I already give a strong reason for my decision. Let say that my 
controller only has space for 4 devices and one of them is offline during 
pre_assign_dyn_addr() and hence it fails. When device tries to do the HJ 
I won't be able to do the enumeration because there is no space left. 
Anyway, you are right and I need to add this to commit message.

BTW, It is not clear to me why should we keep non addressed devices 
allocated when we have i3c_dev_boardinfo list with ->of_node information 
and what is the problem with allocate/free/re-allocate dance?

Best regards,
Vitor Soares




[Index of Archives]     [Device Tree Compilter]     [Device Tree Spec]     [Linux Driver Backports]     [Video for Linux]     [Linux USB Devel]     [Linux PCI Devel]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [XFree86]     [Yosemite Backpacking]


  Powered by Linux