Skip to content

Usb host msd block device #4230

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 3 commits into from
May 26, 2017
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
122 changes: 81 additions & 41 deletions features/unsupported/USBHost/USBHostMSD/USBHostMSD.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,6 @@
#include "USBHostMSD.h"

#if USBHOST_MSD

#include "dbg.h"

#define CBW_SIGNATURE 0x43425355
Expand All @@ -29,13 +28,16 @@
#define GET_MAX_LUN (0xFE)
#define BO_MASS_STORAGE_RESET (0xFF)

USBHostMSD::USBHostMSD(const char * rootdir) : FATFileSystem(rootdir)
USBHostMSD::USBHostMSD()
Copy link
Contributor

@0xc0170 0xc0170 May 8, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

for changes like this, please note in the commit and in the PR description that this is a breaking change? Any change of API should be emphasised and provided with a strong reason for doing so.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The BlockDevice was not existing when the initial version has been developped.
When BlockDevice and fatfilesystem evolution has been introduced, USBHostMSD was no more working (USB: MSD Device #4131) .
Since USB stick is exchanging block write , block read on USB.
To let application using fatfilesystem on this BlockDevice seems in line with the mbed filesystem evolution.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I understand. That's OK.

My intention for these type of changes is that a commit should contain information about breaking the compatibility - this warns all of us that this change is breaking, and any user pulling this commit should be warned. Something nice to have.

{
host = USBHost::getHostInst();
init();
/* register an object in FAT */

init_usb();
}

void USBHostMSD::init() {
void USBHostMSD::init_usb()
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why is this renamed? Is it because of init method conflict somewhere in the hierarchy? Can it be resolved differently?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As USBhostMSD inherits from BlockDevice and block device as a virtual method init.
It is better to rename the method relative to USB init , as init_usb . Also this method is protected so the renaming does not modify the behaviour for the user of this class.

{
dev_connected = false;
dev = NULL;
bulk_in = NULL;
Expand Down Expand Up @@ -72,6 +74,11 @@ bool USBHostMSD::connect()
break;

if (msd_device_found) {
/* As this is done in a specific thread
* this lock is taken to avoid to process a disconnection in
* usb process during the device registering */
USBHost::Lock Lock(host);

bulk_in = dev->getEndpoint(msd_intf, BULK_ENDPOINT, IN);
bulk_out = dev->getEndpoint(msd_intf, BULK_ENDPOINT, OUT);

Expand All @@ -80,14 +87,14 @@ bool USBHostMSD::connect()

USB_INFO("New MSD device: VID:%04x PID:%04x [dev: %p - intf: %d]", dev->getVid(), dev->getPid(), dev, msd_intf);
dev->setName("MSD", msd_intf);
host->registerDriver(dev, msd_intf, this, &USBHostMSD::init);
host->registerDriver(dev, msd_intf, this, &USBHostMSD::init_usb);

dev_connected = true;
return true;
}
} //if()
} //for()
init();
init_usb();
return false;
}

Expand All @@ -99,9 +106,9 @@ bool USBHostMSD::connect()
/*virtual*/ bool USBHostMSD::parseInterface(uint8_t intf_nb, uint8_t intf_class, uint8_t intf_subclass, uint8_t intf_protocol) //Must return true if the interface should be parsed
{
if ((msd_intf == -1) &&
(intf_class == MSD_CLASS) &&
(intf_subclass == 0x06) &&
(intf_protocol == 0x50)) {
(intf_class == MSD_CLASS) &&
(intf_subclass == 0x06) &&
(intf_protocol == 0x50)) {
msd_intf = intf_nb;
return true;
}
Expand All @@ -122,13 +129,15 @@ bool USBHostMSD::connect()
}


int USBHostMSD::testUnitReady() {
int USBHostMSD::testUnitReady()
{
USB_DBG("Test unit ready");
return SCSITransfer(NULL, 6, DEVICE_TO_HOST, 0, 0);
}


int USBHostMSD::readCapacity() {
int USBHostMSD::readCapacity()
{
USB_DBG("Read capacity");
uint8_t cmd[10] = {0x25,0,0,0,0,0,0,0,0,0};
uint8_t result[8];
Expand All @@ -142,7 +151,8 @@ int USBHostMSD::readCapacity() {
}


int USBHostMSD::SCSIRequestSense() {
int USBHostMSD::SCSIRequestSense()
{
USB_DBG("Request sense");
uint8_t cmd[6] = {0x03,0,0,0,18,0};
uint8_t result[18];
Expand All @@ -151,7 +161,8 @@ int USBHostMSD::SCSIRequestSense() {
}


int USBHostMSD::inquiry(uint8_t lun, uint8_t page_code) {
int USBHostMSD::inquiry(uint8_t lun, uint8_t page_code)
{
USB_DBG("Inquiry");
uint8_t evpd = (page_code == 0) ? 0 : 1;
uint8_t cmd[6] = {0x12, uint8_t((lun << 5) | evpd), page_code, 0, 36, 0};
Expand All @@ -174,7 +185,8 @@ int USBHostMSD::inquiry(uint8_t lun, uint8_t page_code) {
return status;
}

int USBHostMSD::checkResult(uint8_t res, USBEndpoint * ep) {
int USBHostMSD::checkResult(uint8_t res, USBEndpoint * ep)
{
// if ep stalled: send clear feature
if (res == USB_TYPE_STALL_ERROR) {
res = host->controlWrite( dev,
Expand All @@ -194,7 +206,8 @@ int USBHostMSD::checkResult(uint8_t res, USBEndpoint * ep) {
}


int USBHostMSD::SCSITransfer(uint8_t * cmd, uint8_t cmd_len, int flags, uint8_t * data, uint32_t transfer_len) {
int USBHostMSD::SCSITransfer(uint8_t * cmd, uint8_t cmd_len, int flags, uint8_t * data, uint32_t transfer_len)
{

int res = 0;

Expand Down Expand Up @@ -277,7 +290,8 @@ int USBHostMSD::SCSITransfer(uint8_t * cmd, uint8_t cmd_len, int flags, uint8_t
}


int USBHostMSD::dataTransfer(uint8_t * buf, uint32_t block, uint8_t nbBlock, int direction) {
int USBHostMSD::dataTransfer(uint8_t * buf, uint32_t block, uint8_t nbBlock, int direction)
{
uint8_t cmd[10];
memset(cmd,0,10);
cmd[0] = (direction == DEVICE_TO_HOST) ? 0x28 : 0x2A;
Expand All @@ -293,20 +307,20 @@ int USBHostMSD::dataTransfer(uint8_t * buf, uint32_t block, uint8_t nbBlock, int
return SCSITransfer(cmd, 10, direction, buf, blockSize*nbBlock);
}

int USBHostMSD::getMaxLun() {
int USBHostMSD::getMaxLun()
{
uint8_t buf[1], res;
res = host->controlRead( dev, USB_RECIPIENT_INTERFACE | USB_DEVICE_TO_HOST | USB_REQUEST_TYPE_CLASS,
0xfe, 0, msd_intf, buf, 1);
USB_DBG("max lun: %d", buf[0]);
return res;
}

int USBHostMSD::disk_initialize() {
int USBHostMSD::init()
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

thanks for fixing the style. I would recommend sending this as separate PR, this is causing distraction from the actual changes in this PR - would be easier if its a separate patch

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I ll take it account in my next PR. (the fact that USB is not in good style does not help ...)

{
USB_DBG("FILESYSTEM: init");
uint16_t i, timeout = 10;

uint16_t i, timeout = 10, ret;
getMaxLun();

for (i = 0; i < timeout; i++) {
Thread::wait(100);
if (!testUnitReady())
Expand All @@ -323,44 +337,70 @@ int USBHostMSD::disk_initialize() {
return readCapacity();
}

int USBHostMSD::disk_write(const uint8_t* buffer, uint32_t block_number, uint32_t count) {
USB_DBG("FILESYSTEM: write block: %lld, count: %d", block_number, count);
int USBHostMSD::program(const void *buffer, bd_addr_t addr, bd_size_t size)
{
uint32_t block_number, count;
uint8_t *buf = (uint8_t *)buffer;
if (!disk_init) {
disk_initialize();
init();
}
if (!disk_init)
if (!disk_init) {
return -1;
}
block_number = addr / blockSize;
count = size /blockSize;

for (uint32_t b = block_number; b < block_number + count; b++) {
if (dataTransfer((uint8_t*)buffer, b, 1, HOST_TO_DEVICE))
if (dataTransfer(buf, b, 1, HOST_TO_DEVICE))
return -1;
buffer += 512;
buf += blockSize;
}
return 0;
}

int USBHostMSD::disk_read(uint8_t* buffer, uint32_t block_number, uint32_t count) {
USB_DBG("FILESYSTEM: read block: %lld, count: %d", block_number, count);
int USBHostMSD::read(void *buffer, bd_addr_t addr, bd_size_t size)
{
uint32_t block_number, count;
uint8_t *buf = (uint8_t *)buffer;
if (!disk_init) {
disk_initialize();
init();
}
if (!disk_init)
if (!disk_init) {
return -1;
}
block_number = addr / blockSize;
count = size / blockSize;

for (uint32_t b = block_number; b < block_number + count; b++) {
if (dataTransfer((uint8_t*)buffer, b, 1, DEVICE_TO_HOST))
if (dataTransfer(buf, b, 1, DEVICE_TO_HOST))
return -1;
buffer += 512;
buf += blockSize;
}
return 0;
}

uint32_t USBHostMSD::disk_sectors() {
USB_DBG("FILESYSTEM: sectors");
if (!disk_init) {
disk_initialize();
}
if (!disk_init)
return 0;
return blockCount;
int USBHostMSD::erase(bd_addr_t addr, bd_size_t size)
{
return 0;
}

bd_size_t USBHostMSD::get_read_size() const
{
return (disk_init ? (bd_size_t)blockSize : -1);
}

bd_size_t USBHostMSD::get_program_size() const
{
return (disk_init ? (bd_size_t)blockSize : -1);
}
bd_size_t USBHostMSD::get_erase_size() const
{
return (disk_init ? (bd_size_t)blockSize : -1);
}

bd_size_t USBHostMSD::size() const
{
USB_DBG("FILESYSTEM: size ");
return (disk_init ? (bd_size_t)blockSize : 0);
}
#endif
45 changes: 27 additions & 18 deletions features/unsupported/USBHost/USBHostMSD/USBHostMSD.h
Original file line number Diff line number Diff line change
Expand Up @@ -23,24 +23,26 @@

#include "USBHost.h"
#include "FATFileSystem.h"
#include "BlockDevice.h"

/**
* A class to communicate a USB flash disk
*/
class USBHostMSD : public IUSBEnumerator, public FATFileSystem {
class USBHostMSD : public IUSBEnumerator, public BlockDevice
{
public:
/**
* Constructor
*
* @param rootdir mount name
*/
USBHostMSD(const char * rootdir);
* Constructor
*
* @param rootdir mount name
*/
USBHostMSD();

/**
* Check if a MSD device is connected
*
* @return true if a MSD device is connected
*/
* Check if a MSD device is connected
*
* @return true if a MSD device is connected
*/
bool connected();

/**
Expand All @@ -49,20 +51,27 @@ class USBHostMSD : public IUSBEnumerator, public FATFileSystem {
* @return true if connection was successful
*/
bool connect();
virtual int init();
virtual int deinit()
{
return BD_ERROR_OK;
};
virtual int read(void *buffer, bd_addr_t addr, bd_size_t size);
virtual int program(const void *buffer, bd_addr_t addr, bd_size_t size);
virtual int erase(bd_addr_t addr, bd_size_t size);
virtual bd_size_t get_read_size() const;
virtual bd_size_t get_program_size() const;
virtual bd_size_t get_erase_size() const;
virtual bd_size_t size() const;



protected:
//From IUSBEnumerator
virtual void setVidPid(uint16_t vid, uint16_t pid);
virtual bool parseInterface(uint8_t intf_nb, uint8_t intf_class, uint8_t intf_subclass, uint8_t intf_protocol); //Must return true if the interface should be parsed
virtual bool useEndpoint(uint8_t intf_nb, ENDPOINT_TYPE type, ENDPOINT_DIRECTION dir); //Must return true if the endpoint will be used

// From FATFileSystem
virtual int disk_initialize();
virtual int disk_status() {return 0;};
virtual int disk_read(uint8_t* buffer, uint32_t sector, uint32_t count);
virtual int disk_write(const uint8_t* buffer, uint32_t sector, uint32_t count);
virtual int disk_sync() {return 0;};
virtual uint32_t disk_sectors();

private:
USBHost * host;
Expand Down Expand Up @@ -110,7 +119,7 @@ class USBHostMSD : public IUSBEnumerator, public FATFileSystem {
bool msd_device_found;
bool disk_init;

void init();
void init_usb();

};

Expand Down
Loading