Re: [PATCH v2 1/2] Phonet: Implement Pipe Controller to support Nokia Slim Modems

["Rémi Denis-Courmont" <remi@remlab.net>]

   Hello,

On Thursday 30 September 2010, Kumar SANGHVI wrote:
> Hi Rémi Denis-Courmont,
> 
> On Wed, Sep 29, 2010 at 20:21:17 +0200, Rémi Denis-Courmont wrote:
> > It seems to me that you really want to implement the connect() socket
> > call, so that one of the two endpoints will stand up for the missing
> > controller.
> 
> Yes, implementing connect() socket call would be nice.
> 
> > That's
> > still much cleaner than CREATE and DESTROY ioctl()'s.
> 
> I have not introduced any new ioctl()'s as part of Pipe controller
> implementation.

Sure. What you did is basically worse than ioctl()'s. You've implemented them 
as socket options. Socket options are meant to configure parameters with 
setsockopt and read paramters with getsockopt. They are not meant for 'doing' 
things - that's what ioctl()'s are for.

> The PIPE_CREATE/PIPE_DESTROY/PIPE_ENABLE/PIPE_DISABLE are all provided
> as socket options.
> So, user-space can call setsockopt for creating/enabling or
> disabling/destroying pipe.

That makes absolutely no sense if you consider how setsockopt and getsockopt 
are supposed to work.

> Regarding implementing connect() socket call, few queries:
> 1. It should carry out all the same steps which I am currently doing as
> part of PIPE_CREATE socket option, right?
> 2. Currently, as part of Pipe controller implementation, user-space
>    follows below sequence:-
> 	socket()
> 	bind()
> 	listen()
> 	setsockopt(PIPE_CREATE)
> 	accept()'
> 
>    In the phonet stack pipe controller logic, we wait for PEP_CONNECT_RESP
>    from host-pep (GPRS socket or video telephony socket is a host-pep.
>    pep_reply sends out the PEP_CONNECT_RESP) and remote-pep (modem),
>    negotiate the best flow-control to be used, and then send
>    PIPE_CREATED_IND, with selected flow-control to both pipe end-points.

connect() should replace listen(), PIPE_CREATE and accept().

-- 
Rémi Denis-Courmont
http://www.remlab.net/