Re: [PATCH v3 1/5] sc_phy:SmartCard(SC) PHY interface to SC controller
On 5/29/2014 12:23 AM, Greg KH wrote: On Wed, May 28, 2014 at 02:27:13PM +0530, Satish Patel wrote: SmartCard controller uses this interface to communicate with SmartCard via PHY Some SmartCard PHY has multiple slots for cards. This inerface also enables controller to communicate with one or more SmartCard connected over phy. interface structure includes following APIs - set/get config - activate/deactivate smart card - warm reset - register_notify (for card insert/remove/overheat) - unregister_notify Signed-off-by: Satish Patel satish.pa...@ti.com --- Documentation/sc_phy.txt | 171 ++ include/linux/sc_phy.h | 136 2 files changed, 307 insertions(+) create mode 100644 Documentation/sc_phy.txt create mode 100644 include/linux/sc_phy.h These are .h files, but where is the api functions that use these structures defined at? This is like template/wrappers, smart card phy driver will write API functions. And smartcard controller will call these functions. With proposed approach, smartcard controller can communicate with any smart card phy (TI/NxP) without change in code. Using DT entry smartcard and PHY will gets connected with each other. Refer diagram given @Documentation/sc_phy.txt. confused, greg k-h -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v3 1/5] sc_phy:SmartCard(SC) PHY interface to SC controller
On 5/29/2014 12:14 AM, Greg KH wrote:
On Wed, May 28, 2014 at 02:27:13PM +0530, Satish Patel wrote:
+/**
+ * struct sc_phy - The basic smart card phy structure
+ *
+ * @dev: phy device
+ * @pdata: pointer to phy's private data structure
+ * @set_config: called to set phy's configuration
+ * @get_config: called to get phy's configuration
+ * @activate_card: perform smart card activation
+ * @deactivate_card: perform smart card de-activation
+ * @warm_reset: execute smart card warm reset sequence
+ * @register_card_activity_cb: register call back to phy device.
+ * This call back will be called on card insert or remove event
+ *
+ * smart card controller uses this interface to communicate with
+ * smart card via phy.Some smart card phy has multiple slots for
+ * cards. This inerface also enables controller to communicate with
+ * one or more smart card connected over phy.
+ */
+struct sc_phy {
+ /* phy's device pointer */
+ struct device *dev;
So this is the parent, right? Why not embed a struct device into this
structure as well, further streaching out the device tree.
Do you mean to use dev-p for pdata ?
I have kept it outside, to give easeness/pragmatic focal for new phy
driver development. Driver can make this pointer null and use above pointer.
+
+ /* phy's private data */
+ void *pdata;
If you do the above, then this pointer is not needed.
+
+ /* notify data, passed by interface user as a part of
+* register_notify API. Data should be passed back when
+* notification raised to the interface user
+*/
+ void *notify_data;
What makes this different from the pdata?
pdata is phy's private data, while notify_data is something phy will
send to smart card controller on some event, like card
remove/insert/timeout etc..
+
+ int (*set_config)(struct sc_phy *phy, u8 slot,
+ enum sc_phy_config attr, int value);
+ int (*get_config)(struct sc_phy *phy, u8 slot, enum
+ sc_phy_config attr);
+ int (*activate_card)(struct sc_phy *phy, u8 slot);
+ int (*deactivate_card)(struct sc_phy *phy, u8 slot);
+ int (*get_syncatr)(struct sc_phy *phy, u8 slot, u8 len, char *atr);
+ int (*warm_reset)(struct sc_phy *phy, u8 slot);
+ int (*register_notify)(struct sc_phy *phy,
+struct notifier_block *nb, void *notify_data);
+ int (*unregister_notify)(struct sc_phy *phy,
+ struct notifier_block *nb);
+};
+
+#endif /* __SC_PHY_H__ */
--
1.7.9.5
--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v3 1/5] sc_phy:SmartCard(SC) PHY interface to SC controller
On Thu, May 29, 2014 at 3:34 AM, Satish Patel satish.pa...@ti.com wrote: On 5/29/2014 12:23 AM, Greg KH wrote: On Wed, May 28, 2014 at 02:27:13PM +0530, Satish Patel wrote: SmartCard controller uses this interface to communicate with SmartCard via PHY Some SmartCard PHY has multiple slots for cards. This inerface also enables controller to communicate with one or more SmartCard connected over phy. interface structure includes following APIs - set/get config - activate/deactivate smart card - warm reset - register_notify (for card insert/remove/overheat) - unregister_notify Signed-off-by: Satish Patel satish.pa...@ti.com --- Documentation/sc_phy.txt | 171 ++ include/linux/sc_phy.h | 136 2 files changed, 307 insertions(+) create mode 100644 Documentation/sc_phy.txt create mode 100644 include/linux/sc_phy.h These are .h files, but where is the api functions that use these structures defined at? This is like template/wrappers, smart card phy driver will write API functions. And smartcard controller will call these functions. With proposed approach, smartcard controller can communicate with any smart card phy (TI/NxP) without change in code. Using DT entry smartcard and PHY will gets connected with each other. Refer diagram given @Documentation/sc_phy.txt. confused, I believe the api Greg is wondering about is the notifier which as I commented is not a good design. There is now a phy subsystem. I don't know if it has what you need, but you should look at it to determine if it will work or could be extended to work. Rob -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v3 1/5] sc_phy:SmartCard(SC) PHY interface to SC controller
On Thu, May 29, 2014 at 02:26:55PM +0530, Satish Patel wrote:
On 5/29/2014 12:14 AM, Greg KH wrote:
On Wed, May 28, 2014 at 02:27:13PM +0530, Satish Patel wrote:
+/**
+ * struct sc_phy - The basic smart card phy structure
+ *
+ * @dev: phy device
+ * @pdata: pointer to phy's private data structure
+ * @set_config: called to set phy's configuration
+ * @get_config: called to get phy's configuration
+ * @activate_card: perform smart card activation
+ * @deactivate_card: perform smart card de-activation
+ * @warm_reset: execute smart card warm reset sequence
+ * @register_card_activity_cb: register call back to phy device.
+ * This call back will be called on card insert or remove event
+ *
+ * smart card controller uses this interface to communicate with
+ * smart card via phy.Some smart card phy has multiple slots for
+ * cards. This inerface also enables controller to communicate with
+ * one or more smart card connected over phy.
+ */
+struct sc_phy {
+ /* phy's device pointer */
+ struct device *dev;
So this is the parent, right? Why not embed a struct device into this
structure as well, further streaching out the device tree.
Do you mean to use dev-p for pdata ?
No, use the device itself.
I have kept it outside, to give easeness/pragmatic focal for new phy driver
development. Driver can make this pointer null and use above pointer.
Ick, no, that's not how the driver model works. Each device in the
system needs a struct device, don't try to chain off of an existing
device like this. Make it a real one.
+
+ /* phy's private data */
+ void *pdata;
If you do the above, then this pointer is not needed.
+
+ /* notify data, passed by interface user as a part of
+* register_notify API. Data should be passed back when
+* notification raised to the interface user
+*/
+ void *notify_data;
What makes this different from the pdata?
pdata is phy's private data, while notify_data is something phy will send to
smart card controller on some event, like card remove/insert/timeout etc..
That doesn't make much sense to me, why not just put that in the notify
function callback itself?
Please either use the existing phy layer of the kernel, or make a real
subsystem here, don't try to put a tiny shim on top of an existing
struct device for this driver, that's not how subsystems in Linux are
done.
thanks,
greg k-h
--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v3 1/5] sc_phy:SmartCard(SC) PHY interface to SC controller
On Thu, May 29, 2014 at 08:47:31AM -0500, Rob Herring wrote: On Thu, May 29, 2014 at 3:34 AM, Satish Patel satish.pa...@ti.com wrote: On 5/29/2014 12:23 AM, Greg KH wrote: On Wed, May 28, 2014 at 02:27:13PM +0530, Satish Patel wrote: SmartCard controller uses this interface to communicate with SmartCard via PHY Some SmartCard PHY has multiple slots for cards. This inerface also enables controller to communicate with one or more SmartCard connected over phy. interface structure includes following APIs - set/get config - activate/deactivate smart card - warm reset - register_notify (for card insert/remove/overheat) - unregister_notify Signed-off-by: Satish Patel satish.pa...@ti.com --- Documentation/sc_phy.txt | 171 ++ include/linux/sc_phy.h | 136 2 files changed, 307 insertions(+) create mode 100644 Documentation/sc_phy.txt create mode 100644 include/linux/sc_phy.h These are .h files, but where is the api functions that use these structures defined at? This is like template/wrappers, smart card phy driver will write API functions. And smartcard controller will call these functions. With proposed approach, smartcard controller can communicate with any smart card phy (TI/NxP) without change in code. Using DT entry smartcard and PHY will gets connected with each other. Refer diagram given @Documentation/sc_phy.txt. confused, I believe the api Greg is wondering about is the notifier which as I commented is not a good design. That, and the fact that if this really is an api, there are no .c files for it like a normal api is in the kernel. There is now a phy subsystem. I don't know if it has what you need, but you should look at it to determine if it will work or could be extended to work. I agree. Satish, what's wrong with our existing phy layer? greg k-h -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v3 1/5] sc_phy:SmartCard(SC) PHY interface to SC controller
On 5/29/2014 7:17 PM, Rob Herring wrote: On Thu, May 29, 2014 at 3:34 AM, Satish Patel satish.pa...@ti.com wrote: On 5/29/2014 12:23 AM, Greg KH wrote: On Wed, May 28, 2014 at 02:27:13PM +0530, Satish Patel wrote: SmartCard controller uses this interface to communicate with SmartCard via PHY Some SmartCard PHY has multiple slots for cards. This inerface also enables controller to communicate with one or more SmartCard connected over phy. interface structure includes following APIs - set/get config - activate/deactivate smart card - warm reset - register_notify (for card insert/remove/overheat) - unregister_notify Signed-off-by: Satish Patel satish.pa...@ti.com --- Documentation/sc_phy.txt | 171 ++ include/linux/sc_phy.h | 136 2 files changed, 307 insertions(+) create mode 100644 Documentation/sc_phy.txt create mode 100644 include/linux/sc_phy.h These are .h files, but where is the api functions that use these structures defined at? This is like template/wrappers, smart card phy driver will write API functions. And smartcard controller will call these functions. With proposed approach, smartcard controller can communicate with any smart card phy (TI/NxP) without change in code. Using DT entry smartcard and PHY will gets connected with each other. Refer diagram given @Documentation/sc_phy.txt. confused, I believe the api Greg is wondering about is the notifier which as I commented is not a good design. There is now a phy subsystem. I don't know if it has what you need, but you should look at it to determine if it will work or could be extended to work. I have given my comments on notifier, it is required to notify real time events like card insert/remove to the smart card controller. As this interrupts tied to phy (in case phy is present) not with controller. Existing phy subsystem does not support generic operations for the phy. If at all it adds supports for these, in future I am ok to get align with it. Rob -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v3 1/5] sc_phy:SmartCard(SC) PHY interface to SC controller
On 5/29/2014 9:21 PM, Greg KH wrote:
On Thu, May 29, 2014 at 02:26:55PM +0530, Satish Patel wrote:
On 5/29/2014 12:14 AM, Greg KH wrote:
On Wed, May 28, 2014 at 02:27:13PM +0530, Satish Patel wrote:
+/**
+ * struct sc_phy - The basic smart card phy structure
+ *
+ * @dev: phy device
+ * @pdata: pointer to phy's private data structure
+ * @set_config: called to set phy's configuration
+ * @get_config: called to get phy's configuration
+ * @activate_card: perform smart card activation
+ * @deactivate_card: perform smart card de-activation
+ * @warm_reset: execute smart card warm reset sequence
+ * @register_card_activity_cb: register call back to phy device.
+ * This call back will be called on card insert or remove event
+ *
+ * smart card controller uses this interface to communicate with
+ * smart card via phy.Some smart card phy has multiple slots for
+ * cards. This inerface also enables controller to communicate with
+ * one or more smart card connected over phy.
+ */
+struct sc_phy {
+ /* phy's device pointer */
+ struct device *dev;
So this is the parent, right? Why not embed a struct device into this
structure as well, further streaching out the device tree.
Do you mean to use dev-p for pdata ?
No, use the device itself.
I have kept it outside, to give easeness/pragmatic focal for new phy driver
development. Driver can make this pointer null and use above pointer.
Ick, no, that's not how the driver model works. Each device in the
system needs a struct device, don't try to chain off of an existing
device like this. Make it a real one.
+
+ /* phy's private data */
+ void *pdata;
If you do the above, then this pointer is not needed.
+
+ /* notify data, passed by interface user as a part of
+* register_notify API. Data should be passed back when
+* notification raised to the interface user
+*/
+ void *notify_data;
What makes this different from the pdata?
pdata is phy's private data, while notify_data is something phy will send to
smart card controller on some event, like card remove/insert/timeout etc..
That doesn't make much sense to me, why not just put that in the notify
function callback itself?
Little correction over here..
Here is brief flow
USIM(Smart Card Controller) register notification callback to smarcard phy.
usim-phy-register_notify(phy handle, callback fn, notify_data);
notify_data : pass back when callback function will be called by PHY
Smartcard PHY - USIM
blocking_notifier_call_chain(notifier, action, notify_data);
action : card insert/remove/overheat etc..
notify data: USIM data passing back (supplied at the time of cb
registration)
Please either use the existing phy layer of the kernel, or make a real
subsystem here, don't try to put a tiny shim on top of an existing
struct device for this driver, that's not how subsystems in Linux are
done.
Why I am not using exiting PHY framework ?
We will be more than happy to adapt generic phy if it
includes/add support for smart card requirements like
1) Some smart card phy (TDA8026-NxP) has multiple slots for smart
cards. This interface enables controller to communicate with specific
smart card inserted to the specific phy's slot.
2) Warm reset to smart card inserted to phy slot.
3) Bit banging of smart card pins to support vendor specific memory
cards.
4) Notification of card insert/remove/overheat etc.
5) synchronous and asynchronous modes for smart card transaction
thanks,
greg k-h
--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v3 1/5] sc_phy:SmartCard(SC) PHY interface to SC controller
On Wed, May 28, 2014 at 02:27:13PM +0530, Satish Patel wrote:
+/**
+ * struct sc_phy - The basic smart card phy structure
+ *
+ * @dev: phy device
+ * @pdata: pointer to phy's private data structure
+ * @set_config: called to set phy's configuration
+ * @get_config: called to get phy's configuration
+ * @activate_card: perform smart card activation
+ * @deactivate_card: perform smart card de-activation
+ * @warm_reset: execute smart card warm reset sequence
+ * @register_card_activity_cb: register call back to phy device.
+ * This call back will be called on card insert or remove event
+ *
+ * smart card controller uses this interface to communicate with
+ * smart card via phy.Some smart card phy has multiple slots for
+ * cards. This inerface also enables controller to communicate with
+ * one or more smart card connected over phy.
+ */
+struct sc_phy {
+ /* phy's device pointer */
+ struct device *dev;
So this is the parent, right? Why not embed a struct device into this
structure as well, further streaching out the device tree.
+
+ /* phy's private data */
+ void *pdata;
If you do the above, then this pointer is not needed.
+
+ /* notify data, passed by interface user as a part of
+ * register_notify API. Data should be passed back when
+ * notification raised to the interface user
+ */
+ void *notify_data;
What makes this different from the pdata?
+
+ int (*set_config)(struct sc_phy *phy, u8 slot,
+ enum sc_phy_config attr, int value);
+ int (*get_config)(struct sc_phy *phy, u8 slot, enum
+ sc_phy_config attr);
+ int (*activate_card)(struct sc_phy *phy, u8 slot);
+ int (*deactivate_card)(struct sc_phy *phy, u8 slot);
+ int (*get_syncatr)(struct sc_phy *phy, u8 slot, u8 len, char *atr);
+ int (*warm_reset)(struct sc_phy *phy, u8 slot);
+ int (*register_notify)(struct sc_phy *phy,
+ struct notifier_block *nb, void *notify_data);
+ int (*unregister_notify)(struct sc_phy *phy,
+ struct notifier_block *nb);
+};
+
+#endif /* __SC_PHY_H__ */
--
1.7.9.5
--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v3 1/5] sc_phy:SmartCard(SC) PHY interface to SC controller
On Wed, May 28, 2014 at 02:27:13PM +0530, Satish Patel wrote: SmartCard controller uses this interface to communicate with SmartCard via PHY Some SmartCard PHY has multiple slots for cards. This inerface also enables controller to communicate with one or more SmartCard connected over phy. interface structure includes following APIs - set/get config - activate/deactivate smart card - warm reset - register_notify (for card insert/remove/overheat) - unregister_notify Signed-off-by: Satish Patel satish.pa...@ti.com --- Documentation/sc_phy.txt | 171 ++ include/linux/sc_phy.h | 136 2 files changed, 307 insertions(+) create mode 100644 Documentation/sc_phy.txt create mode 100644 include/linux/sc_phy.h These are .h files, but where is the api functions that use these structures defined at? confused, greg k-h -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
