Skip to content
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

Usbd MSC windows minimum scsi implementation #47

Open
wants to merge 7 commits into
base: master
Choose a base branch
from

Conversation

alexrayne
Copy link

here provided MSC scsi , commands that denamded by win8

also introduced API that allow overriding by user code.

this branch also includes USBD logging refactorings to USBD_LOGXXX macros, vs old simple LOGXXX.

also level of log verbosity defined by value of USBD_DEBUG. levels system and loging sections are declared in usbd_private.h

*     usbd_private.h:USBD_LOGxxx(verbose_level - migrate to new debug printer style
+                    USB_Vxxx - denotes verbosity levels
+                    USB_VCALL,VIO,VSETUP - denotes code sections to log
*     usbd_log_call - deploy USBD_LOG_CALL to standalone printer routine
*     usbd_dwc_otg - starts migrate to new logstyle
…LBA adress

+               usbd_msc_blocks - extraxt this method for future backend changes
…by windows msc driver

+         msc_scsi.h - extrat scsi api to alone header
+usb/byteorder.h  - provide hton/ntoh convertions for scsi api
…and <read format capacity> on windows.

+usbd:msc:usbd_msc_backend:inquiry_page - introduce INQUIRY into msc backend.
+                          unit_blocks_count - introduce api for media size in backend
+         usbd_scsi_inquiry_(page|standart_page) - publish default implementations of inquiry for external override
+         usbd_scsi_inquiry_evpd_(supports|serial) - publish default page composers for external override
@danielinux danielinux requested a review from kuldeepdhaka May 20, 2017 08:40
@@ -92,6 +92,8 @@ struct sbc_sense_info {
uint8_t ascq;
};


#define USBD_MSC_SEC_SIZE 512
Copy link
Contributor

Choose a reason for hiding this comment

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

Good idea 👍

@@ -64,15 +64,17 @@ typedef struct usbd_msc_backend usbd_msc_backend;
* @param lock Lock. Optional - can be NULL
* @param unlock Unlock. Optional - can be NULL
*/
typedef uint32_t msc_lba_t;
Copy link
Contributor

Choose a reason for hiding this comment

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

Look ok to me but,

Tab between "uint32_t" "msc_lba_t;"
(Not nitpicking) but a space is sufficient.
In other part of the code (was a typedef), you use a space instead.
Please read more code of the project and maintain the same pattern/code style.

} scsi_periferial_qualify;

typedef enum {
, scsi_dtDirectBlock = 0 //* < Direct access block device (e.g., magnetic disk)
Copy link
Contributor

Choose a reason for hiding this comment

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

  • C Comment
  • "," before enum decl
  • enum are expected to be uppercase

set_sbc_status_good(ms);
}
}

Copy link
Contributor

Choose a reason for hiding this comment

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

What are you trying to do with the new code.
please explain the purpose.
a comment or commit msg should be ok.

Copy link
Author

Choose a reason for hiding this comment

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

the name of routine scsi_read_format_capacities - not enough? this is standart scsi request. win demand it from usb msd.

{
if (EVENT_CBW_VALID == event) {
usb_msc_rfc_capacity_list_header* h = (usb_msc_rfc_capacity_list_header*)trans->msd_buf;
usb_msc_rfc_capacity_descriptor* descr = (usb_msc_rfc_capacity_descriptor*) (trans->msd_buf+sizeof(*h));
Copy link
Contributor

Choose a reason for hiding this comment

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

"trans->msd_buf+sizeof(*h)" space before & after "+".

enum trans_event event)
{
if (EVENT_CBW_VALID == event) {
usb_msc_rfc_capacity_list_header* h = (usb_msc_rfc_capacity_list_header*)trans->msd_buf;
Copy link
Contributor

Choose a reason for hiding this comment

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

"(usb_msc_rfc_capacity_list_header*)trans->msd_buf" space after "*)"

@kuldeepdhaka
Copy link
Contributor

Note to myself:

  • Need to look into the MSC new code. (what it does and is it implementing correctly)
  • the logging code is being push with MSC code, make them two different PR because they are decoupled & im still not sure if we are going in right directly for logging API.

LGPL License Terms @ref lgpl_license
*/
/*
* Copyright (C) 2016 alexrayne <[email protected]>
Copy link
Contributor

Choose a reason for hiding this comment

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

Minor note: INAL, as of my understanding of copyright.
copyright name/year/email for Weston Schmidt and/or Pavol Rusnak need to be added because they authored the file from which some of the content you moved.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants