[PATCH v9 0/8] Generic PHY Framework

[Felipe Balbi]

Hi,

On Tue, Jul 09, 2013 at 02:33:58PM +0200, Patel, Satish wrote:
> > > Operation:
> > >
> > > Once the card gets inserted into the slot, user application can
> > > initiate activate sequence anytime, it is not mandatory that
> > > activation has to be called Immediately. It all depends whether
> > other
> > > connections ( network, printer) are proper.  User application can do
> > > some checks and then dynamically activate the card
> > 
> > The card or the phy ? Who actually activates ? Is the activation
> > handling part of the PHY or part of the smartcard controller ?
> > 
> 
> It’s a activation of smart card which is inserted into the phy slot. 
> Smart card activation will by done phy but initiated by user application
> Handling of activation should be done at both the side - controller as well
> As phy. Phy will raise certain signals and check timeout while activation
> , while controller will store data coming from smart card to FIFO. Its
> Controller which will give command to phy to initiate activation sequence when it
> is ready. 
> 
> Which means, both controller and phy has role to play while activation.
> Also it is applicable while deactivation.

right, so when the smartcard device is opened, you want to powerup the
PHY, powerup the smartcard controller and get things going, which means
that all your activation logic can, and should, be hidden by
phy_power_on() and phy_power_off()

Perhaps you also want to delay phy_init() and phy_shutdown() to that
very time, which would mean that we wouldn't even need
phy_power_{on,off}() calls whatsoever.

> > > There are 2 sequence of activation - normal activation and warm
> > reset.
> > > If first fails then warm reset should be executed, if card does not
> > > response to both then deactivation will be carried out
> > 
> > ret = phy_resume(phy);
> > if (ret == -EINVAL) { /* perhaps there's a better error code */
> > 	ret = phy_reset(phy);
> > 	if (ret)
> > 		phy_suspend(phy);
> > }
> >
> 
> Its not like this. There are the cases, when during normal activation
> card respond With some characters, but they might not be valid or
> error prone - This check will be done by user application as only
> application knows how to decode characters as per protocol.

sure, and if the characters are invalid, the application needs to tell
the kernel, perhaps it would be enough to send and IOCTL or, even
easier, close the device and reopen it.

> It there is any error in characters/timeout paratmeters, user
> application will ask to initiate Warm reset.

and for that we can use an IOCTL which is implemented by smart card
layer. I mean, I'm assuming the smartcard would be seen as a char dev by
userland so it would be something like this:

ret = read(fd, buf, SIZE);
if (ret < 0)
	boom();

ret = process_buffer(buf, ret);
if (ret == -EINVAL) { /* we have received invalid characters */
	ret = ioctl(fd, IOCTL_SMARTCARD_WARM_RESET);
	if (ret < 0)
		handle_error();
}

in kernel space we would have an ioctl handler like so:

static int smart_card_ioctl(struct file *fd, unsigned code, usigned long v)
{
	struct smart_card *sc = fd->private_data;
	unsigned long flags;
	int ret;

	spin_lock_irqsave(&sc->lock, flags);
	switch (v)
	case SMART_CARD_WARM_RESET:
		ret = phy_shutdown(sc->phy);
		if (ret < 0)
			boom();

		ret = smart_card_reset(sc);
		if (ret < 0)
			boom();

		ret = phy_init(sc->phy);
		if (ret < 0)
			boom();
		break;
	[ ... ]
	}

	spin_unlock_irqrestore(&sc->lock, flags);

	return ret;
}

see, we don't need to add a bunch of stuff in the PHY framework, because
you shouldn't expose the PHY directly to userland. You *must* have an
abstraction to userland with an in-kernel usage of the PHY layer.

> > > - on activation card will respond with some initial characters
> > (ATR),
> > > which will be verified by application, in case of error user
> > > application will de-activate the card not the phy module
> > >
> > > - After activation, use application initiate normal tx/rx
> > >
> > > - Once tx/rx is done, it will initiate deactivation of card not the
> > > phy module.
> > 
> > then that has nothing to do with the PHY framework, right ?
> 
> But all such sequence will be initiated by PHY by raising certain
> signals and only controller can give the push when to start.

sure, that's fair. But you don't need to expose every single detail of
the PHY. You need to use abstractions to make accesses to the PHY easy
to maintain. See the example above.

> > > > > Smartcard_set_c4/c8/rst/io -> phy_set_pin
> > > >
> > > > Whats should be exactly done here? Looks to me like it should be
> > > > part of init. Does these pin settings need to be changed
> > dynamically?
> > >
> > > These are all phy pins which will be physically come in contact with
> > > smart card When inserted. In case of synchronous card, these pins
> > will
> > > be toggled manually by s/w bit bang algorithm
> > 
> > is that a requirement or just something that everybody does ?
> >
> 
> Its requirement for synchronous card. At present there is no open driver
> Which does this thing.

alright, so you need to tell the PHY in which mode it should work and
the PHY driver needs to be smart enough to choose proper callbacks
depending on the mode.

> > In any case, you can hide that under some gpio calls, or something
> > similar.
> 
> Can't be done using GPIO as pin can be toggled only which i2c read/write
> to the phy.

so ? look at the numerous I2C GPIO expanders we have in the kernel ;-)

> > > > > Smartcard_get_version -> phy_get_version
> > > >
> > > > Again why? Why would the smartcard need the version of the PHY?
> > > >
> > >
> > > Can be omitted. This is for the test suite reference.
> > 
> > if it's for testsuite, it sounds like something that should be done
> > through userland and, as such, should be implemented by a set of
> > debugfs
> > files which expose the necessary information. That has nothing to do
> > with the PHY framework.
> > 
> Agree.. can be omitted.
> 
> 
> I am ok to give internal webex presentation (TI team) to brief 
> on phy and smart card controller interface.

let's try to cut down on internal agreements and let the community help
defining a proper way to handle the PHY :-)

Still, if you want to point me and Kishon (privately) to some internal
documentation, we can certainly read through it :-)

cheers

-- 
balbi
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 836 bytes
Desc: Digital signature
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20130709/9c264c9a/attachment.sig>